Merge pull request #2591 from thomasvl/objc_timestamps_take2

Timestamp helper fix, Duration helper cleanup.

objectivec/GPBWellKnownTypes.h

@@ -112,16 +112,27 @@ typedef NS_ENUM(NSInteger, GPBWellKnownTypesErrorCode) {
  * @note: Not all second/nanos combinations can be represented in a
  * NSTimeInterval, so getting this could be a lossy transform.
  **/
-@property(nonatomic, readwrite) NSTimeInterval timeIntervalSince1970;
+@property(nonatomic, readwrite) NSTimeInterval timeInterval;
 
 /**
  * Initializes a GPBDuration with the given NSTimeInterval.
  *
- * @param timeIntervalSince1970 Time interval to configure the GPBDuration with.
+ * @param timeInterval Time interval to configure the GPBDuration with.
  *
  * @return A newly initialized GPBDuration.
  **/
-- (instancetype)initWithTimeIntervalSince1970:(NSTimeInterval)timeIntervalSince1970;
+- (instancetype)initWithTimeInterval:(NSTimeInterval)timeInterval;
+
+// These next two methods are deprecated because GBPDuration has no need of a
+// "base" time. The older methods were about symmetry with GBPTimestamp, but
+// the unix epoch usage is too confusing.
+
+/** Deprecated, use timeInterval instead. */
+@property(nonatomic, readwrite) NSTimeInterval timeIntervalSince1970
+    __attribute__((deprecated("Use timeInterval")));
+/** Deprecated, use initWithTimeInterval: instead. */
+- (instancetype)initWithTimeIntervalSince1970:(NSTimeInterval)timeIntervalSince1970
+    __attribute__((deprecated("Use initWithTimeInterval:")));
 
 @end
 

objectivec/GPBWellKnownTypes.m

@@ -41,15 +41,25 @@ NSString *const GPBWellKnownTypesErrorDomain =
 
 static NSString *kTypePrefixGoogleApisCom = @"type.googleapis.com/";
 
-static NSTimeInterval TimeIntervalSince1970FromSecondsAndNanos(int64_t seconds,
-                                                               int32_t nanos) {
+static NSTimeInterval TimeIntervalFromSecondsAndNanos(int64_t seconds,
+                                                      int32_t nanos) {
   return seconds + (NSTimeInterval)nanos / 1e9;
 }
 
-static int32_t SecondsAndNanosFromTimeIntervalSince1970(NSTimeInterval time,
-                                                        int64_t *outSeconds) {
+static int32_t SecondsAndNanosFromTimeInterval(NSTimeInterval time,
+                                               int64_t *outSeconds,
+                                               BOOL nanosMustBePositive) {
   NSTimeInterval seconds;
   NSTimeInterval nanos = modf(time, &seconds);
+
+  if (nanosMustBePositive && (nanos < 0)) {
+    // Per Timestamp.proto, nanos is non-negative and "Negative second values with
+    // fractions must still have non-negative nanos values that count forward in
+    // time. Must be from 0 to 999,999,999 inclusive."
+    --seconds;
+    nanos = 1.0 + nanos;
+  }
+
   nanos *= 1e9;
   *outSeconds = (int64_t)seconds;
   return (int32_t)nanos;
@@ -88,8 +98,8 @@ static NSString *ParseTypeFromURL(NSString *typeURLString) {
 - (instancetype)initWithTimeIntervalSince1970:(NSTimeInterval)timeIntervalSince1970 {
   if ((self = [super init])) {
     int64_t seconds;
-    int32_t nanos = SecondsAndNanosFromTimeIntervalSince1970(
-        timeIntervalSince1970, &seconds);
+    int32_t nanos = SecondsAndNanosFromTimeInterval(
+        timeIntervalSince1970, &seconds, YES);
     self.seconds = seconds;
     self.nanos = nanos;
   }
@@ -105,13 +115,13 @@ static NSString *ParseTypeFromURL(NSString *typeURLString) {
 }
 
 - (NSTimeInterval)timeIntervalSince1970 {
-  return TimeIntervalSince1970FromSecondsAndNanos(self.seconds, self.nanos);
+  return TimeIntervalFromSecondsAndNanos(self.seconds, self.nanos);
 }
 
 - (void)setTimeIntervalSince1970:(NSTimeInterval)timeIntervalSince1970 {
   int64_t seconds;
   int32_t nanos =
-      SecondsAndNanosFromTimeIntervalSince1970(timeIntervalSince1970, &seconds);
+      SecondsAndNanosFromTimeInterval(timeIntervalSince1970, &seconds, YES);
   self.seconds = seconds;
   self.nanos = nanos;
 }
@@ -122,29 +132,41 @@ static NSString *ParseTypeFromURL(NSString *typeURLString) {
 
 @implementation GPBDuration (GBPWellKnownTypes)
 
-- (instancetype)initWithTimeIntervalSince1970:(NSTimeInterval)timeIntervalSince1970 {
+- (instancetype)initWithTimeInterval:(NSTimeInterval)timeInterval {
   if ((self = [super init])) {
     int64_t seconds;
-    int32_t nanos = SecondsAndNanosFromTimeIntervalSince1970(
-        timeIntervalSince1970, &seconds);
+    int32_t nanos = SecondsAndNanosFromTimeInterval(
+        timeInterval, &seconds, NO);
     self.seconds = seconds;
     self.nanos = nanos;
   }
   return self;
 }
 
-- (NSTimeInterval)timeIntervalSince1970 {
-  return TimeIntervalSince1970FromSecondsAndNanos(self.seconds, self.nanos);
+- (instancetype)initWithTimeIntervalSince1970:(NSTimeInterval)timeIntervalSince1970 {
+  return [self initWithTimeInterval:timeIntervalSince1970];
 }
 
-- (void)setTimeIntervalSince1970:(NSTimeInterval)timeIntervalSince1970 {
+- (NSTimeInterval)timeInterval {
+  return TimeIntervalFromSecondsAndNanos(self.seconds, self.nanos);
+}
+
+- (void)setTimeInterval:(NSTimeInterval)timeInterval {
   int64_t seconds;
   int32_t nanos =
-      SecondsAndNanosFromTimeIntervalSince1970(timeIntervalSince1970, &seconds);
+      SecondsAndNanosFromTimeInterval(timeInterval, &seconds, NO);
   self.seconds = seconds;
   self.nanos = nanos;
 }
 
+- (NSTimeInterval)timeIntervalSince1970 {
+  return self.timeInterval;
+}
+
+- (void)setTimeIntervalSince1970:(NSTimeInterval)timeIntervalSince1970 {
+  self.timeInterval = timeIntervalSince1970;
+}
+
 @end
 
 #pragma mark - GPBAny

objectivec/Tests/GPBWellKnownTypesTest.m

@@ -32,11 +32,9 @@
 
 #import <XCTest/XCTest.h>
 
+#import "GPBTestUtilities.h"
 #import "google/protobuf/AnyTest.pbobjc.h"
 
-// A basically random interval into the future for testing with.
-static const NSTimeInterval kFutureOffsetInterval = 15000;
-
 // Nanosecond time accuracy
 static const NSTimeInterval kTimeAccuracy = 1e-9;
 
@@ -46,59 +44,117 @@ static const NSTimeInterval kTimeAccuracy = 1e-9;
 @implementation WellKnownTypesTest
 
 - (void)testTimeStamp {
-  // Test Creation.
-  NSDate *date = [NSDate date];
-  GPBTimestamp *timeStamp = [[GPBTimestamp alloc] initWithDate:date];
-  NSDate *timeStampDate = timeStamp.date;
-
-  // Comparing timeIntervals instead of directly comparing dates because date
-  // equality requires the time intervals to be exactly the same, and the
-  // timeintervals go through a bit of floating point error as they are
-  // converted back and forth from the internal representation.
-  XCTAssertEqualWithAccuracy(date.timeIntervalSince1970,
-                             timeStampDate.timeIntervalSince1970,
-                             kTimeAccuracy);
-
-  NSTimeInterval time = [date timeIntervalSince1970];
-  GPBTimestamp *timeStamp2 =
-      [[GPBTimestamp alloc] initWithTimeIntervalSince1970:time];
-  NSTimeInterval durationTime = timeStamp2.timeIntervalSince1970;
-  XCTAssertEqualWithAccuracy(time, durationTime, kTimeAccuracy);
-  [timeStamp release];
-
-  // Test Mutation.
-  date = [NSDate dateWithTimeIntervalSinceNow:kFutureOffsetInterval];
-  timeStamp2.date = date;
-  timeStampDate = timeStamp2.date;
-  XCTAssertEqualWithAccuracy(date.timeIntervalSince1970,
-                             timeStampDate.timeIntervalSince1970,
-                             kTimeAccuracy);
-
-  time = date.timeIntervalSince1970;
-  timeStamp2.timeIntervalSince1970 = time;
-  durationTime = timeStamp2.timeIntervalSince1970;
-  XCTAssertEqualWithAccuracy(time, durationTime, kTimeAccuracy);
-  [timeStamp2 release];
+  // Test negative and positive values.
+  NSTimeInterval values[] = {
+      -428027599.483999967, -1234567.0, -0.5, 0, 0.75, 54321.0, 2468086,483999967
+  };
+  for (size_t i = 0; i < GPBARRAYSIZE(values); ++i) {
+    NSTimeInterval value = values[i];
+
+    // Test Creation - date.
+    NSDate *date = [NSDate dateWithTimeIntervalSince1970:value];
+    GPBTimestamp *timeStamp = [[GPBTimestamp alloc] initWithDate:date];
+
+    XCTAssertGreaterThanOrEqual(timeStamp.nanos, 0,
+                                @"Offset %f - Date: %@", (double)value, date);
+    XCTAssertLessThan(timeStamp.nanos, 1e9,
+                      @"Offset %f - Date: %@", (double)value, date);
+
+    // Comparing timeIntervals instead of directly comparing dates because date
+    // equality requires the time intervals to be exactly the same, and the
+    // timeintervals go through a bit of floating point error as they are
+    // converted back and forth from the internal representation.
+    XCTAssertEqualWithAccuracy(value, timeStamp.date.timeIntervalSince1970,
+                               kTimeAccuracy,
+                               @"Offset %f - Date: %@", (double)value, date);
+    [timeStamp release];
+
+    // Test Creation - timeIntervalSince1970.
+    timeStamp = [[GPBTimestamp alloc] initWithTimeIntervalSince1970:value];
+
+    XCTAssertGreaterThanOrEqual(timeStamp.nanos, 0,
+                                @"Offset %f - Date: %@", (double)value, date);
+    XCTAssertLessThan(timeStamp.nanos, 1e9,
+                      @"Offset %f - Date: %@", (double)value, date);
+
+    XCTAssertEqualWithAccuracy(value, timeStamp.timeIntervalSince1970,
+                               kTimeAccuracy,
+                               @"Offset %f - Date: %@", (double)value, date);
+    [timeStamp release];
+
+    // Test Mutation - date.
+    timeStamp = [[GPBTimestamp alloc] init];
+    timeStamp.date = date;
+
+    XCTAssertGreaterThanOrEqual(timeStamp.nanos, 0,
+                                @"Offset %f - Date: %@", (double)value, date);
+    XCTAssertLessThan(timeStamp.nanos, 1e9,
+                      @"Offset %f - Date: %@", (double)value, date);
+
+    XCTAssertEqualWithAccuracy(value, timeStamp.date.timeIntervalSince1970,
+                               kTimeAccuracy,
+                               @"Offset %f - Date: %@", (double)value, date);
+    [timeStamp release];
+
+    // Test Mutation - timeIntervalSince1970.
+    timeStamp = [[GPBTimestamp alloc] init];
+    timeStamp.timeIntervalSince1970 = value;
+
+    XCTAssertGreaterThanOrEqual(timeStamp.nanos, 0,
+                                @"Offset %f - Date: %@", (double)value, date);
+    XCTAssertLessThan(timeStamp.nanos, 1e9,
+                      @"Offset %f - Date: %@", (double)value, date);
+
+    XCTAssertEqualWithAccuracy(value, timeStamp.date.timeIntervalSince1970,
+                               kTimeAccuracy,
+                               @"Offset %f - Date: %@", (double)value, date);
+
+    [timeStamp release];
+  }
 }
 
 - (void)testDuration {
-  // Test Creation.
-  NSTimeInterval time = [[NSDate date] timeIntervalSince1970];
-  GPBDuration *duration =
-      [[GPBDuration alloc] initWithTimeIntervalSince1970:time];
-  NSTimeInterval durationTime = duration.timeIntervalSince1970;
-  XCTAssertEqualWithAccuracy(time, durationTime, kTimeAccuracy);
-  [duration release];
-
-  // Test Mutation.
-  GPBDuration *duration2 =
-      [[GPBDuration alloc] initWithTimeIntervalSince1970:time];
-  NSDate *date = [NSDate dateWithTimeIntervalSinceNow:kFutureOffsetInterval];
-  time = date.timeIntervalSince1970;
-  duration2.timeIntervalSince1970 = time;
-  durationTime = duration2.timeIntervalSince1970;
-  XCTAssertEqualWithAccuracy(time, durationTime, kTimeAccuracy);
-  [duration2 release];
+  // Test negative and positive values.
+  NSTimeInterval values[] = { -1000.0001, -500.0, -0.5, 0, 0.75, 1000.0, 2000.0002 };
+  for (size_t i = 0; i < GPBARRAYSIZE(values); ++i) {
+    NSTimeInterval value = values[i];
+
+    // Test Creation.
+    GPBDuration *duration =
+        [[GPBDuration alloc] initWithTimeInterval:value];
+    XCTAssertEqualWithAccuracy(value, duration.timeInterval, kTimeAccuracy,
+                               @"For interval %f", (double)value);
+    if (value > 0) {
+      XCTAssertGreaterThanOrEqual(duration.seconds, 0,
+                                  @"For interval %f", (double)value);
+      XCTAssertGreaterThanOrEqual(duration.nanos, 0,
+                                  @"For interval %f", (double)value);
+    } else {
+      XCTAssertLessThanOrEqual(duration.seconds, 0,
+                               @"For interval %f", (double)value);
+      XCTAssertLessThanOrEqual(duration.nanos, 0,
+                               @"For interval %f", (double)value);
+    }
+    [duration release];
+
+    // Test Mutation.
+    duration = [[GPBDuration alloc] init];
+    duration.timeInterval = value;
+    XCTAssertEqualWithAccuracy(value, duration.timeInterval, kTimeAccuracy,
+                               @"For interval %f", (double)value);
+    if (value > 0) {
+      XCTAssertGreaterThanOrEqual(duration.seconds, 0,
+                                  @"For interval %f", (double)value);
+      XCTAssertGreaterThanOrEqual(duration.nanos, 0,
+                                  @"For interval %f", (double)value);
+    } else {
+      XCTAssertLessThanOrEqual(duration.seconds, 0,
+                               @"For interval %f", (double)value);
+      XCTAssertLessThanOrEqual(duration.nanos, 0,
+                               @"For interval %f", (double)value);
+    }
+    [duration release];
+  }
 }
 
 - (void)testAnyHelpers {