test-results: bug fixes and tests for TestFile Put, PutData

go/src/infra/appengine/test-results/model/test_file.go

 package model
 
 import (
         "bytes"
         "fmt"
         "io"
         "io/ioutil"
         "math"
         "time"
 
         "golang.org/x/net/context"
 
         "github.com/luci/gae/service/datastore"
         "github.com/luci/luci-go/common/errors"
         "github.com/luci/luci-go/common/logging"
 )
 
 // IsAggregateTestFile returns whether filename is that of an aggregate test file.
 func IsAggregateTestFile(filename string) bool {
         switch filename {
         case "results.json", "results-small.json":
                 return true
         default:
                 return false
         }
 }
 
 // BuildNum is int64 that is used to handle TestFile datastore records
 // with null build_number. The value is >= 0 if the datastore value
-// was not null.
+// was not null. The value is -1 for null datastore value.
 type BuildNum int64
 
 var _ datastore.PropertyConverter = (*BuildNum)(nil)
 
 // IsNil returns whether b had null value in datastore.
 func (b *BuildNum) IsNil() bool { return *b == -1 }
 
 // ToProperty is for implementing datastore.PropertyConverter.
 func (b *BuildNum) ToProperty() (p datastore.Property, err error) {
         if b.IsNil() {
@@ skipping 17 matching lines @@
 }
 
 // DataEntry represents a DataEntry record.
 type DataEntry struct {
         ID   int64  `gae:"$id"`
         Data []byte `gae:"data,noindex"`
 }
 
 // TestFile represents a TestFile record.
 type TestFile struct {
-        ID          int64            `gae:"$id"`
-        BuildNumber BuildNum         `gae:"build_number"`
-        Builder     string           `gae:"builder"`
-        DataKeys    []*datastore.Key `gae:"data_keys,noindex"`
-        LastMod     time.Time        `gae:"date"`
-        Master      string           `gae:"master"`
-        Name        string           `gae:"name"`
-        TestType    string           `gae:"test_type"`
+        ID          int64    `gae:"$id"`
+        BuildNumber BuildNum `gae:"build_number"`
+        Builder     string   `gae:"builder"`
+        Master      string   `gae:"master"`
+        Name        string   `gae:"name"`
+        TestType    string   `gae:"test_type"`
+
+        // DataKeys is the keys to the DataEntry(s) that contain
+        // the data for this TestFile.
+        DataKeys []*datastore.Key `gae:"data_keys,noindex"`
+
+        // LastMod is the last modified time.
+        LastMod time.Time `gae:"date"`
 
         // Data is the data in the DataEntry(s) pointed to by DataKeys.
         // After loading a TestFile from the datastore, this field is
         // only available after GetData is called. To put updated
         // data in this field to the datastore, call PutData.
         //
-        // Users should typically perform the following sequence of calls
+        // Users will typically perform the following sequence of calls
         // in a transaction:
         //
-        //   - datastore.Get(tf)
-        //   - tf.GetData()
-        //   - tf.PutData()
-        //   - datastore.Put(tf)
-        //   - datastore.Delete(tf.OldDataKeys)
+        //   ds = datastore.Get(ctx)
+        //   err = ds.Get(tf)
+        //   err = tf.GetData(ctx)
+        //   // manipulate tf.Data
+        //   err = tf.PutData(ctx)
+        //   err = ds.Put(tf)
+        //   err = ds.Delete(tf.OldDataKeys)
         //
         Data io.Reader `gae:"-,noindex"`
 
         // OldDataKeys is the keys of the old DataEntry(s) used
         // before putting the new data. This field is set after a
         // successful call to PutData.
         //
         // Users are responsible for deleting the DataEntry(s)
         // pointed to by these keys if they are no longer needed.
         OldDataKeys []*datastore.Key `gae:"-,noindex"`
 
-        // newDataKeys is unused in this implementation. It is
+        // NewDataKeys is UNUSED in this implementation. It is
         // a remnant of the old Python implementation.
-        newDataKeys []*datastore.Key `gae:"new_data_keys,noindex"`
+        NewDataKeys []*datastore.Key `gae:"new_data_keys,noindex"`
 }
 
 // GetData fetches data from the DataEntry(s) pointed to by tf.DataKeys
 // and sets tf.Data to the fetched data.
 func (tf *TestFile) GetData(c context.Context) error {
         ids := make([]int64, len(tf.DataKeys))
         for i, dk := range tf.DataKeys {
                 ids[i] = dk.IntID() // Intentional: ignores the Kind stored in dk.
         }
         ids, err := updatedBlobKeys(c, ids...)
@@ skipping 46 matching lines @@
 // tf.Data and updates tf.LastMod and tf.DataKeys locally.
 // If the returned error is non-nil, tf will be unmodified,
 // except that tf.Data may have been consumed.
 func (tf *TestFile) putDataEntries(c context.Context) error {
         // Maximum data entries in a TestFile.
         const maxDataEntries = 30
         // 1 megabyte is the maximum allowed blob length.
         // See https://code.googlesource.com/gocloud/+/master/datastore/prop.go#29.
         const maxBlobLen = 1 << 20
 
-        // TODO: Read maxBlobLen bytes at a time. See io.LimitedReader.
+        // TODO(maybe): Read maxBlobLen bytes at a time. See io.LimitedReader.
 
         data, err := ioutil.ReadAll(tf.Data)
         if err != nil {
                 return err
         }
 
         if len(data) > maxDataEntries*maxBlobLen {
                 return fmt.Errorf(
                         "model: data too large %d bytes (max allowed %d bytes)",
                         len(data),
@@ skipping 12 matching lines @@
         if err := datastore.Get(c).Put(dataEntries); err != nil {
                 return err
         }
 
         newKeys := make([]*datastore.Key, 0, len(dataEntries))
         for _, de := range dataEntries {
                 newKeys = append(newKeys, datastore.Get(c).KeyForObj(&de))
         }
 
         tf.DataKeys = newKeys
-        tf.LastMod = time.Now()
+        tf.LastMod = time.Now().UTC()
         return nil
 }
 
 // updatedBlobKeys fetches the updated blob keys for the old keys from their blobstore migration
 // records. len(n) will equal len(old) if there is no error. If len(old)>1 and there is an error,
 // err will be of type MultiError and the error at index i will correspond to the
 // error in retrieving the updated blob key for the key at index i in old. A
 // nil error indicates that all values returned in n are valid.
 func updatedBlobKeys(c context.Context, old ...int64) (n []int64, err error) {
         ds := datastore.Get(c)
@@ skipping 103 matching lines @@
         q = q.Order("-date")
 
         if p.Limit < 0 || p.Limit > defaultLimit {
                 q = q.Limit(defaultLimit)
         } else {
                 q = q.Limit(p.Limit)
         }
 
         return q
 }