Code review comment for lp:~rogpeppe/gozk/comments-and-time

Reviewers: mp+92750_code.launchpad.net,

Message:
Please take a look.

Description:
Also change Stat.CTime and Stat.MTime to return time.Time.

https://code.launchpad.net/~rogpeppe/gozk/comments-and-time/+merge/92750

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/5650075/

Affected files:
   M zk.go
   M zk_test.go

Index: zk.go
=== <email address hidden> >
<email address hidden>
=== modified file 'zk.go'
--- zk.go 2012-02-08 14:34:24 +0000
+++ zk.go 2012-02-13 12:38:16 +0000
@@ -92,8 +92,17 @@
  // event.Type is set to EVENT_CLOSED and event.State is set to
STATE_CLOSED,
  // to facilitate handling.
  type Event struct {
- Type int
- Path string
+ // Type gives the type of event (one of the EVENT_* constants).
+ // If Type is EVENT_SESSION, then the event is a session
+ // event.
+ Type int
+
+ // For non-session events, Path gives the path of the node
+ // that was being watched.
+ Path string
+
+ // For session events, State (one of the STATE* constants) gives the
session
+ // status.
   State int
  }

@@ -304,46 +313,64 @@
   c C.struct_Stat
  }

+// Czxid returns the zxid of the change that caused the node to be created.
  func (stat *Stat) Czxid() int64 {
   return int64(stat.c.czxid)
  }

+// Mzxid returns the zxid of the change that last modified the node.
  func (stat *Stat) Mzxid() int64 {
   return int64(stat.c.mzxid)
  }

-func (stat *Stat) CTime() int64 {
- return int64(stat.c.ctime)
-}
-
-func (stat *Stat) MTime() int64 {
- return int64(stat.c.mtime)
-}
-
+func millisec2time(ms int64) time.Time {
+ return time.Unix(ms/1e3, ms%1e3*1e6)
+}
+
+// CTime returns the time (at millisecond resolution) when the node was
+// created.
+func (stat *Stat) CTime() time.Time {
+ return millisec2time(int64(stat.c.ctime))
+}
+
+// MTime returns the time (at millisecond resolution) when the node was
+// last modified.
+func (stat *Stat) MTime() time.Time {
+ return millisec2time(int64(stat.c.mtime))
+}
+
+// Version returns the number of changes to the data of the node.
  func (stat *Stat) Version() int32 {
   return int32(stat.c.version)
  }

+// CVersion returns the number of changes to the children of the node.
  func (stat *Stat) CVersion() int32 {
   return int32(stat.c.cversion)
  }

+// AVersion returns the number of changes to the ACL of the node.
  func (stat *Stat) AVersion() int32 {
   return int32(stat.c.aversion)
  }

+// If the node is an ephemeral node, EphemeralOwner returns the session id
+// of the owner of the node; otherwise it will return zero.
  func (stat *Stat) EphemeralOwner() int64 {
   return int64(stat.c.ephemeralOwner)
  }

+// DataLength returns the length of the data in the node in bytes.
  func (stat *Stat) DataLength() int32 {
   return int32(stat.c.dataLength)
  }

+// NumChildren returns the number of children of the znode.
  func (stat *Stat) NumChildren() int32 {
   return int32(stat.c.numChildren)
  }

+// Pzxid returns the Pzxid of the node, whatever that is.
  func (stat *Stat) Pzxid() int64 {
   return int64(stat.c.pzxid)
  }

Index: zk_test.go
=== <email address hidden> >
<email address hidden>
=== modified file 'zk_test.go'
--- zk_test.go 2011-12-05 19:15:00 +0000
+++ zk_test.go 2012-02-13 12:36:27 +0000
@@ -140,8 +140,8 @@
   c.Assert(data, Equals, "")
   c.Assert(stat.Czxid(), Equals, int64(0))
   c.Assert(stat.Mzxid(), Equals, int64(0))
- c.Assert(stat.CTime(), Equals, int64(0))
- c.Assert(stat.MTime(), Equals, int64(0))
+ c.Assert(stat.CTime(), Equals, time.Unix(0, 0))
+ c.Assert(stat.MTime(), Equals, time.Unix(0, 0))
   c.Assert(stat.Version(), Equals, int32(0))
   c.Assert(stat.CVersion(), Equals, int32(0))
   c.Assert(stat.AVersion(), Equals, int32(0))
@@ -178,15 +178,33 @@
   c.Assert(data, Equals, "bababum")
  }

+func checkTimeBetween(c *C, what string, t, t0, t1 time.Time) {
+ // Truncate the start time to millisecond resolution, as
+ // time stamps get similarly truncated.
+ t0 = t0.Add(-time.Duration(t0.Nanosecond() % 1e6))
+ maxdt := t1.Sub(t0)
+ dt := t.Sub(t0)
+ if dt < 0 || dt > maxdt {
+ c.Errorf("%s out of range; expected between %v and %v, got %v", what,
t0.Format(time.StampNano), t1.Format(time.StampNano),
t.Format(time.StampNano))
+ }
+}
+
  func (s *S) TestCreateSetAndGet(c *C) {
   conn, _ := s.init(c)

+ start := time.Now()
   _, err := conn.Create("/test", "", zk.EPHEMERAL, zk.WorldACL(zk.PERM_ALL))
   c.Assert(err, IsNil)

- stat, err := conn.Set("/test", "bababum", -1) // Any version.
+ _, stat, err := conn.Get("/test")
+ c.Assert(err, IsNil)
+ checkTimeBetween(c, "ctime", stat.CTime(), start, time.Now())
+
+ start = time.Now()
+ stat, err = conn.Set("/test", "bababum", -1) // Any version.
   c.Assert(err, IsNil)
   c.Assert(stat.Version(), Equals, int32(1))
+ checkTimeBetween(c, "mtime", stat.MTime(), start, time.Now())

   data, _, err := conn.Get("/test")
   c.Assert(err, IsNil)