Correct TimeOfDay type, and add marshalling for time.tz.

soap/types.go

@@ -65,10 +65,17 @@ type TimeOfDay struct {
 	// Duration of time since midnight.
 	FromMidnight time.Duration
 
-	// TimeZone is present only if time.tz is used. It is otherwise ignored.
+	// Set to true if Offset is specified. If false, then the timezone is
-	TimeZone *time.Location
+	// unspecified (and by ISO8601 - implies some "local" time).
+	HasOffset bool
+
+	// Offset is non-zero only if time.tz is used. It is otherwise ignored. If
+	// non-zero, then it is regarded as a UTC offset in seconds. Note that the
+	// sub-minutes is ignored by the marshal function.
+	Offset int16
 }
 
+// MarshalTimeOfDay marshals TimeOfDay to the "time" type.
 func MarshalTimeOfDay(v TimeOfDay) (string, error) {
 	d := int64(v.FromMidnight / time.Second)
 	hour := d / 3600
@@ -79,17 +86,91 @@ func MarshalTimeOfDay(v TimeOfDay) (string, error) {
 	return fmt.Sprintf("%02d:%02d:%02d", hour, minute, second), nil
 }
 
-var timeRegexp = regexp.MustCompile(`^(\d\d)(?::?(\d\d)(?::?(\d\d))?)?$`)
+// UnmarshalTimeOfDay unmarshals TimeOfDay from the "time" type.
-
 func UnmarshalTimeOfDay(s string) (TimeOfDay, error) {
-	parts := timeRegexp.FindStringSubmatch(s)
+	t, err := UnmarshalTimeOfDayTz(s)
-	if len(parts) < 2 {
+	if err != nil {
-		return TimeOfDay{}, fmt.Errorf("soap time: value %q is not in ISO8601 time format", s)
+		return TimeOfDay{}, err
+	} else if t.HasOffset {
+		return TimeOfDay{}, fmt.Errorf("soap time: value %q contains unexpected timezone")
 	}
+	return t, nil
+}
+
+// MarshalTimeOfDayTz marshals TimeOfDay to the "time.tz" type.
+func MarshalTimeOfDayTz(v TimeOfDay) (string, error) {
+	d := int64(v.FromMidnight / time.Second)
+	hour := d / 3600
+	d = d % 3600
+	minute := d / 60
+	second := d % 60
+
+	tz := ""
+	if v.HasOffset {
+		if v.Offset == 0 {
+			tz = "Z"
+		} else {
+			offsetMins := v.Offset / 60
+			sign := '+'
+			if offsetMins < 1 {
+				offsetMins = -offsetMins
+				sign = '-'
+			}
+			tz = fmt.Sprintf("%c%02d:%02d", sign, offsetMins/60, offsetMins%60)
+		}
+	}
+
+	return fmt.Sprintf("%02d:%02d:%02d%s", hour, minute, second, tz), nil
+}
+
+var timeRegexp = regexp.MustCompile(
+	`^(\d\d)(?::?(\d\d)(?::?(\d\d))?)?` + // hh[:mm[:ss]]
+		`(?:(Z)|([+-])(\d\d)(?::?(\d\d))?)?$`) // Z | ±hh[:mm]
+
+// UnmarshalTimeOfDayTz unmarshals TimeOfDay from the "time.tz" type.
+func UnmarshalTimeOfDayTz(s string) (TimeOfDay, error) {
+	parts := timeRegexp.FindStringSubmatch(s)
+	if parts == nil {
+		return TimeOfDay{}, fmt.Errorf("soap time.tz: value %q is not in ISO8601 time format", s)
+	}
+
+	// HH:MM:SS parsing.
 	parts = parts[1:]
 	var iParts [3]int64
-	for i, pStr := range parts {
+	for i, pStr := range parts[:3] {
 		iParts[i], _ = strconv.ParseInt(pStr, 10, 64)
 	}
-	return TimeOfDay{time.Duration(iParts[0]*3600+iParts[1]*60+iParts[2]) * time.Second, nil}, nil
+	hour, minute, second := iParts[0], iParts[1], iParts[2]
+
+	fromMidnight := time.Duration(hour*3600+minute*60+second) * time.Second
+
+	// ISO8601 special case - values up to 24:00:00 are allowed, so using
+	// strictly greater-than for the maximum value.
+	if fromMidnight > 24*time.Hour || minute >= 60 || second >= 60 {
+		return TimeOfDay{}, fmt.Errorf("soap time.tz: value %q has value(s) out of range", s)
+	}
+
+	// Timezone offset parsing.
+	hasOffset := false
+	var offset int64
+	if parts[3] == "Z" {
+		hasOffset = true
+		offset = 0
+	} else if parts[4] != "" {
+		hasOffset = true
+		hours, _ := strconv.ParseInt(parts[5], 10, 64)
+		var mins int64
+		if parts[6] != "" {
+			mins, _ = strconv.ParseInt(parts[6], 10, 64)
+		}
+		offset = hours*3600 + mins*60
+		if parts[4] == "-" {
+			offset = -offset
+		}
+	}
+	return TimeOfDay{
+		FromMidnight: time.Duration(hour*3600+minute*60+second) * time.Second,
+		HasOffset:    hasOffset,
+		Offset:       int16(offset),
+	}, nil
 }

soap/types_test.go

@@ -19,6 +19,7 @@ type testCase struct {
 	wantUnmarshalErr bool
 	noMarshal        bool
 	noUnMarshal      bool
+	tag              string
 }
 
 type Fixed14_4Test float64
@@ -71,7 +72,26 @@ func (v TimeOfDayTest) Equal(result interface{}) bool {
 	return v.TimeOfDay == result.(TimeOfDay)
 }
 
+type TimeOfDayTzTest struct {
+	TimeOfDay
+}
+
+func (v TimeOfDayTzTest) Marshal() (string, error) {
+	return MarshalTimeOfDayTz(v.TimeOfDay)
+}
+func (v TimeOfDayTzTest) Unmarshal(s string) (interface{}, error) {
+	return UnmarshalTimeOfDayTz(s)
+}
+func (v TimeOfDayTzTest) Equal(result interface{}) bool {
+	return v.TimeOfDay == result.(TimeOfDay)
+}
+
 func Test(t *testing.T) {
+	const time010203 time.Duration = (1*3600 + 2*60 + 3) * time.Second
+	const time0102 time.Duration = (1*3600 + 2*60) * time.Second
+	const time01 time.Duration = (1 * 3600) * time.Second
+	const time235959 time.Duration = (23*3600 + 59*60 + 59) * time.Second
+
 	tests := []testCase{
 		// Fixed14_4
 		{str: "0.0000", value: Fixed14_4Test(0)},
@@ -99,39 +119,79 @@ func Test(t *testing.T) {
 		{str: "-1", value: DateTest{}, wantUnmarshalErr: true, noMarshal: true},
 
 		// Time
-		{str: "00:00:00", value: TimeOfDayTest{TimeOfDay{0, nil}}},
+		{str: "00:00:00", value: TimeOfDayTest{TimeOfDay{FromMidnight: 0}}},
-		{str: "000000", value: TimeOfDayTest{TimeOfDay{0, nil}}, noMarshal: true},
+		{str: "000000", value: TimeOfDayTest{TimeOfDay{FromMidnight: 0}}, noMarshal: true},
-		{str: "01:02:03", value: TimeOfDayTest{TimeOfDay{(1*3600 + 2*60 + 3) * time.Second, nil}}},
+		{str: "24:00:00", value: TimeOfDayTest{TimeOfDay{FromMidnight: 24 * time.Hour}}, noMarshal: true}, // ISO8601 special case
-		{str: "010203", value: TimeOfDayTest{TimeOfDay{(1*3600 + 2*60 + 3) * time.Second, nil}}, noMarshal: true},
+		{str: "24:01:00", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true},
-		{str: "23:59:59", value: TimeOfDayTest{TimeOfDay{(23*3600 + 59*60 + 59) * time.Second, nil}}},
+		{str: "24:00:01", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true},
-		{str: "235959", value: TimeOfDayTest{TimeOfDay{(23*3600 + 59*60 + 59) * time.Second, nil}}, noMarshal: true},
+		{str: "25:00:00", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true},
-		{str: "01:02", value: TimeOfDayTest{TimeOfDay{(1*3600 + 2*60) * time.Second, nil}}, noMarshal: true},
+		{str: "00:60:00", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true},
-		{str: "0102", value: TimeOfDayTest{TimeOfDay{(1*3600 + 2*60) * time.Second, nil}}, noMarshal: true},
+		{str: "00:00:60", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true},
-		{str: "01", value: TimeOfDayTest{TimeOfDay{(1 * 3600) * time.Second, nil}}, noMarshal: true},
+		{str: "01:02:03", value: TimeOfDayTest{TimeOfDay{FromMidnight: time010203}}},
-		{str: "01", value: TimeOfDayTest{TimeOfDay{(1 * 3600) * time.Second, nil}}, noMarshal: true},
+		{str: "010203", value: TimeOfDayTest{TimeOfDay{FromMidnight: time010203}}, noMarshal: true},
-		{str: "foo 01:02:03", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true},
+		{str: "23:59:59", value: TimeOfDayTest{TimeOfDay{FromMidnight: time235959}}},
-		{str: "foo\n01:02:03", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true},
+		{str: "235959", value: TimeOfDayTest{TimeOfDay{FromMidnight: time235959}}, noMarshal: true},
-		{str: "01:02:03 foo", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true},
+		{str: "01:02", value: TimeOfDayTest{TimeOfDay{FromMidnight: time0102}}, noMarshal: true},
-		{str: "01:02:03\nfoo", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true},
+		{str: "0102", value: TimeOfDayTest{TimeOfDay{FromMidnight: time0102}}, noMarshal: true},
+		{str: "01", value: TimeOfDayTest{TimeOfDay{FromMidnight: time01}}, noMarshal: true},
+		{str: "foo 01:02:03", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true, tag: "notz"},
+		{str: "foo\n01:02:03", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true, tag: "notz"},
+		{str: "01:02:03 foo", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true, tag: "notz"},
+		{str: "01:02:03\nfoo", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true, tag: "notz"},
+		{str: "01:02:03Z", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true, tag: "notz"},
+		{str: "01:02:03+01", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true, tag: "notz"},
+		{str: "01:02:03+01:23", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true, tag: "notz"},
+		{str: "01:02:03+0123", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true, tag: "notz"},
+		{str: "01:02:03-01", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true, tag: "notz"},
+		{str: "01:02:03-01:23", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true, tag: "notz"},
+		{str: "01:02:03-0123", value: TimeOfDayTest{}, wantUnmarshalErr: true, noMarshal: true, tag: "notz"},
+
+		// Time.tz
+		{str: "24:00:01", value: TimeOfDayTzTest{}, wantUnmarshalErr: true, noMarshal: true},
+		{str: "01Z", value: TimeOfDayTzTest{TimeOfDay{time01, true, 0}}, noMarshal: true},
+		{str: "01:02:03Z", value: TimeOfDayTzTest{TimeOfDay{time010203, true, 0}}},
+		{str: "01+01", value: TimeOfDayTzTest{TimeOfDay{time01, true, 3600}}, noMarshal: true},
+		{str: "01:02:03+01", value: TimeOfDayTzTest{TimeOfDay{time010203, true, 3600}}, noMarshal: true},
+		{str: "01:02:03+01:23", value: TimeOfDayTzTest{TimeOfDay{time010203, true, 3600 + 23*60}}},
+		{str: "01:02:03+0123", value: TimeOfDayTzTest{TimeOfDay{time010203, true, 3600 + 23*60}}, noMarshal: true},
+		{str: "01:02:03-01", value: TimeOfDayTzTest{TimeOfDay{time010203, true, -3600}}, noMarshal: true},
+		{str: "01:02:03-01:23", value: TimeOfDayTzTest{TimeOfDay{time010203, true, -(3600 + 23*60)}}},
+		{str: "01:02:03-0123", value: TimeOfDayTzTest{TimeOfDay{time010203, true, -(3600 + 23*60)}}, noMarshal: true},
 	}
 
+	var extras []testCase
+	for i, test := range tests {
+		if value, ok := test.value.(TimeOfDayTest); ok && test.tag != "notz" {
+			// Auto-generate Time.tz cases from Time cases where the check isn't
+			// checking for a forbidden timezone offset.
+			test.value = TimeOfDayTzTest{value.TimeOfDay}
+			// Assert that we are working on a copy (just in case future code change
+			// changes tests to a slice of pointers).
+			if _, ok := tests[i].value.(TimeOfDayTest); !ok {
+				t.Fatal("Mutated original TimeOfDayTest in making TimeOfDayTzTest copy.")
+			}
+			extras = append(extras, test)
+		}
+	}
+	tests = append(tests, extras...)
+
 	for _, test := range tests {
 		if test.noMarshal {
 		} else if resultStr, err := test.value.Marshal(); err != nil && !test.wantMarshalErr {
-			t.Errorf("For %s, want %q, got error: %v", test.value, test.str, err)
+			t.Errorf("For %T marshal %v, want %q, got error: %v", test.value, test.value, test.str, err)
 		} else if err == nil && test.wantMarshalErr {
-			t.Errorf("For %s, want error, got %q", test.value, resultStr)
+			t.Errorf("For %T marshal %v, want error, got %q", test.value, test.value, resultStr)
 		} else if err == nil && resultStr != test.str {
-			t.Errorf("For %s, want %q, got %q", test.value, test.str, resultStr)
+			t.Errorf("For %T marshal %v, want %q, got %q", test.value, test.value, test.str, resultStr)
 		}
 
 		if test.noUnMarshal {
 		} else if resultValue, err := test.value.Unmarshal(test.str); err != nil && !test.wantUnmarshalErr {
-			t.Errorf("For %q, want %v, got error: %v", test.str, test.value, err)
+			t.Errorf("For %T unmarshal %q, want %v, got error: %v", test.value, test.str, test.value, err)
 		} else if err == nil && test.wantUnmarshalErr {
-			t.Errorf("For %q, want error, got %v", test.str, resultValue)
+			t.Errorf("For %T unmarshal %q, want error, got %v", test.value, test.str, resultValue)
 		} else if err == nil && !test.value.Equal(resultValue) {
-			t.Errorf("For %q, want %v, got %v", test.str, test.value, resultValue)
+			t.Errorf("For %T unmarshal %q, want %v, got %v", test.value, test.str, test.value, resultValue)
 		}
 	}
 }