3448: unit tests, bug fixes

UnixVolume.Delete tests the return status of lockfile (oops, left out of
previous commit)

New unit/integration tests:

* TestPutTouch (volume_unix_test.go): issues a volume.Put for a block
  that already exists, and confirms that the mtime of the existing block
  is updated.

* TestPutBlockTouchFails (keepstore_test.go): issues PutBlock for a
  block that already exists but cannot be modified, confirms that the
  mtime on the old block is unchanged and that a new copy has been
  stored.

Added a volume.Mtime() method to return the block mtime, to assist in
testing.

diff --git a/services/keepstore/keepstore_test.go b/services/keepstore/keepstore_test.go
index d43a00e153ee52a724f38c911bea9f591ca9eb75..2b7e411aaf1f8c560da93af7fb4c770b60363fde 100644 (file)
--- a/services/keepstore/keepstore_test.go
+++ b/services/keepstore/keepstore_test.go
@@ -217,7 +217,7 @@ func TestPutBlockCorrupt(t *testing.T) {
        }
 }
 
-// PutBlockCollision
+// TestPutBlockCollision
 //     PutBlock returns a 400 Collision error when attempting to
 //     store a block that collides with another block on disk.
 //
@@ -245,6 +245,53 @@ func TestPutBlockCollision(t *testing.T) {
        }
 }
 
+// TestPutBlockTouchFails
+//     When PutBlock is asked to PUT an existing block, but cannot
+//     modify the timestamp, it should write a second block.
+//
+func TestPutBlockTouchFails(t *testing.T) {
+       defer teardown()
+
+       // Prepare two test Keep volumes.
+       KeepVM = MakeTestVolumeManager(2)
+       defer func() { KeepVM.Quit() }()
+       vols := KeepVM.Volumes()
+
+       // Store a block and then make the underlying volume bad,
+       // so a subsequent attempt to update the file timestamp
+       // will fail.
+       vols[0].Put(TEST_HASH, BAD_BLOCK)
+       old_mtime, err := vols[0].Mtime(TEST_HASH)
+       if err != nil {
+               t.Fatalf("vols[0].Mtime(%s): %s\n", TEST_HASH, err)
+       }
+
+       // Mark the volume bad and call PutBlock.
+       vols[0].(*MockVolume).Bad = true
+       if err := PutBlock(TEST_BLOCK, TEST_HASH); err != nil {
+               t.Fatalf("PutBlock: %v", err)
+       }
+       vols[0].(*MockVolume).Bad = false
+
+       // Now the mtime on the block on vols[0] should be unchanged, and
+       // there should be a copy of the block on vols[1].
+       new_mtime, err := vols[0].Mtime(TEST_HASH)
+       if err != nil {
+               t.Fatalf("vols[0].Mtime(%s): %s\n", TEST_HASH, err)
+       }
+       if !new_mtime.Equal(old_mtime) {
+               t.Errorf("bad block mtimes do not match:\nold_mtime = %v\nnew_mtime = %v\n",
+                       old_mtime, new_mtime)
+       }
+       result, err := vols[1].Get(TEST_HASH)
+       if err != nil {
+               t.Fatalf("vols[1]: %v", err)
+       }
+       if bytes.Compare(result, TEST_BLOCK) != 0 {
+               t.Errorf("new block does not match test block\nnew block = %v\n", result)
+       }
+}
+
 // ========================================
 // FindKeepVolumes tests.
 // ========================================

diff --git a/services/keepstore/volume.go b/services/keepstore/volume.go
index 0406c3d1983fc9ce718f20797447d7218ea93276..6fb1a1e0876324aac78046932dbfe511f3384cf4 100644 (file)
--- a/services/keepstore/volume.go
+++ b/services/keepstore/volume.go
@@ -16,6 +16,7 @@ type Volume interface {
        Get(loc string) ([]byte, error)
        Put(loc string, block []byte) error
        Touch(loc string) error
+       Mtime(loc string) (time.Time, error)
        Index(prefix string) string
        Delete(loc string) error
        Status() *VolumeStatus
@@ -66,6 +67,19 @@ func (v *MockVolume) Touch(loc string) error {
        return nil
 }
 
+func (v *MockVolume) Mtime(loc string) (time.Time, error) {
+       var mtime time.Time
+       var err error
+       if v.Bad {
+               err = errors.New("Bad volume")
+       } else if t, ok := v.Timestamps[loc]; ok {
+               mtime = t
+       } else {
+               err = os.ErrNotExist
+       }
+       return mtime, err
+}
+
 func (v *MockVolume) Index(prefix string) string {
        var result string
        for loc, block := range v.Store {

diff --git a/services/keepstore/volume_unix.go b/services/keepstore/volume_unix.go
index f43997860061eb45688dcd9fa28cde40557e2194..75a75229a6861f83f45b9264bab9d577d7d0f880 100644 (file)
--- a/services/keepstore/volume_unix.go
+++ b/services/keepstore/volume_unix.go
@@ -112,6 +112,15 @@ func (v *UnixVolume) Touch(loc string) error {
        return syscall.Utime(p, &utime)
 }
 
+func (v *UnixVolume) Mtime(loc string) (time.Time, error) {
+       p := v.blockPath(loc)
+       if fi, err := os.Stat(p); err != nil {
+               return time.Time{}, err
+       } else {
+               return fi.ModTime(), nil
+       }
+}
+
 // Read retrieves a block identified by the locator string "loc", and
 // returns its contents as a byte slice.
 //
@@ -250,7 +259,9 @@ func (v *UnixVolume) Delete(loc string) error {
        if err != nil {
                return err
        }
-       lockfile(f)
+       if e := lockfile(f); e != nil {
+               return e
+       }
        defer unlockfile(f)
 
        // If the block has been PUT more recently than -permission_ttl,

diff --git a/services/keepstore/volume_unix_test.go b/services/keepstore/volume_unix_test.go
index 278e656066a0adc2ec5d59bb1814c0f04776f00d..7a1c06c8161e0c4c8d9e8a8681aa7a510371643b 100644 (file)
--- a/services/keepstore/volume_unix_test.go
+++ b/services/keepstore/volume_unix_test.go
@@ -100,6 +100,41 @@ func TestPutBadVolume(t *testing.T) {
        }
 }
 
+// TestPutTouch
+//     Test that when applying PUT to a block that already exists,
+//     the block's modification time is updated.
+func TestPutTouch(t *testing.T) {
+       v := TempUnixVolume(t, false)
+       defer _teardown(v)
+
+       if err := v.Put(TEST_HASH, TEST_BLOCK); err != nil {
+               t.Error(err)
+       }
+       old_mtime, err := v.Mtime(TEST_HASH)
+       if err != nil {
+               t.Error(err)
+       }
+       if old_mtime.IsZero() {
+               t.Errorf("v.Mtime(%s) returned a zero mtime\n", TEST_HASH)
+       }
+       // Sleep for 1s, then put the block again.  The volume
+       // should report a more recent mtime.
+       // TODO(twp): this would be better handled with a mock Time object.
+       time.Sleep(time.Second)
+       if err := v.Put(TEST_HASH, TEST_BLOCK); err != nil {
+               t.Error(err)
+       }
+       new_mtime, err := v.Mtime(TEST_HASH)
+       if err != nil {
+               t.Error(err)
+       }
+
+       if !new_mtime.After(old_mtime) {
+               t.Errorf("v.Put did not update the block mtime:\nold_mtime = %v\nnew_mtime = %v\n",
+                       old_mtime, new_mtime)
+       }
+}
+
 // Serialization tests: launch a bunch of concurrent
 //
 // TODO(twp): show that the underlying Read/Write operations executed