Merge pull request #165 from calvinmclean/feature/improve-start-time

Improve StartTime format for WaterSchedules

Taskfile.yml

@@ -2,6 +2,7 @@ version: "3"
 
 tasks:
   integration-test:
+    aliases: ["it"]
     desc: Startup docker resources and run integration tests. Cleanup when done.
     dir: ./garden-app
     cmds:
@@ -12,6 +13,7 @@ tasks:
       - go test -race -covermode=atomic -coverprofile=integration_coverage.out -coverpkg=./... ./integration_tests
 
   unit-test:
+    aliases: ["ut"]
     desc: Run unit tests for Go app
     dir: ./garden-app
     cmds:

garden-app/api/openapi.yaml

@@ -914,8 +914,9 @@ components:
           example: 72h
         start_time:
           type: string
-          format: date-time
+          format: time
           description: time that the watering interval should be started at
+          example: 23:00:00-07:00
         weather_control:
           $ref: "#/components/schemas/WeatherControl"
           description: control watering based on weather data. Requires a configured weather client

garden-app/gardens.yaml.example

@@ -1,10 +1,10 @@
 Garden_chokmn1nhf81274ru2mg: '{"name":"Indoor Seed Starting","topic_prefix":"seed-garden","id":"chokmn1nhf81274ru2mg","max_zones":3,"created_at":"2023-05-27T00:14:20.324Z","light_schedule":{"duration":"16h0m0s","start_time":"06:00:00-07:00"}}'
 Garden_cos1pt0n1e43o39cs40g: '{"name":"Front Yard","topic_prefix":"front-yard","id":"cos1pt0n1e43o39cs40g","max_zones":4,"created_at":"2024-05-05T16:58:10.206246-07:00"}'
-WaterSchedule_chokmq1nhf81274ru2n0: '{"id":"chokmq1nhf81274ru2n0","duration":"1s","interval":"30s","start_time":"2023-05-23T15:00:00Z","end_date":"2024-05-05T17:01:10.335976-07:00","name":"WaterEvery30s"}'
-WaterSchedule_cii72s9nhf8f7gdpckug: '{"id":"cii72s9nhf8f7gdpckug","duration":"1h0m0s","interval":"480h0m0s","start_time":"2023-06-17T12:00:00Z","name":"Winter Trees","description":"Water deeply and infrequently in the winter","active_period":{"start_month":"October","end_month":"March"}}'
-WaterSchedule_cjbg22a8tio6of9s8o0g: '{"id":"cjbg22a8tio6of9s8o0g","duration":"30s","interval":"24h0m0s","start_time":"2023-07-25T15:00:00Z","name":"Seedlings","description":"Water seedlings a bit every day"}'
-WaterSchedule_cos1s28n1e43sc2vb4k0: '{"id":"cos1s28n1e43sc2vb4k0","duration":"1h30m0s","interval":"240h0m0s","start_time":"2023-06-17T12:00:00Z","name":"Summer Trees","description":"Water deeply every 10 days","active_period":{"start_month":"April","end_month":"September"}}'
-WaterSchedule_cos1suon1e43sc2vb4kg: '{"id":"cos1suon1e43sc2vb4kg","duration":"45m0s","interval":"120h0m0s","start_time":"2006-01-02T15:04:05Z","name":"Shrubs","description":"Water shrubs every 5 days"}'
+WaterSchedule_chokmq1nhf81274ru2n0: '{"id":"chokmq1nhf81274ru2n0","duration":"1s","interval":"30s","start_time":"15:00:00Z","end_date":"2024-05-05T17:01:10.335976-07:00","name":"WaterEvery30s"}'
+WaterSchedule_cii72s9nhf8f7gdpckug: '{"id":"cii72s9nhf8f7gdpckug","duration":"1h0m0s","interval":"480h0m0s","start_time":"12:00:00Z","name":"Winter Trees","description":"Water deeply and infrequently in the winter","active_period":{"start_month":"October","end_month":"March"}}'
+WaterSchedule_cjbg22a8tio6of9s8o0g: '{"id":"cjbg22a8tio6of9s8o0g","duration":"30s","interval":"24h0m0s","start_time":"15:00:00Z","name":"Seedlings","description":"Water seedlings a bit every day"}'
+WaterSchedule_cos1s28n1e43sc2vb4k0: '{"id":"cos1s28n1e43sc2vb4k0","duration":"1h30m0s","interval":"240h0m0s","start_time":"12:00:00Z","name":"Summer Trees","description":"Water deeply every 10 days","active_period":{"start_month":"April","end_month":"September"}}'
+WaterSchedule_cos1suon1e43sc2vb4kg: '{"id":"cos1suon1e43sc2vb4kg","duration":"45m0s","interval":"120h0m0s","start_time":"15:04:05Z","name":"Shrubs","description":"Water shrubs every 5 days"}'
 Zone_chokn19nhf81274ru2o0: |
     {
       "name": "Zone 1",

garden-app/go.mod

@@ -4,6 +4,7 @@ go 1.21.3
 
 require (
 	github.com/AlecAivazis/survey/v2 v2.3.7
+	github.com/ajg/form v1.5.1
 	github.com/calvinmclean/babyapi v0.14.0
 	github.com/eclipse/paho.mqtt.golang v1.4.3
 	github.com/go-chi/render v1.0.3
@@ -31,7 +32,6 @@ require (
 	github.com/FZambia/sentinel v1.1.1 // indirect
 	github.com/Joker/jade v1.1.3 // indirect
 	github.com/Shopify/goreferrer v0.0.0-20220729165902-8cddb4f5de06 // indirect
-	github.com/ajg/form v1.5.1 // indirect
 	github.com/andybalholm/brotli v1.0.6 // indirect
 	github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect
 	github.com/aymerick/douceur v0.2.0 // indirect

garden-app/integration_tests/main_test.go

@@ -161,14 +161,14 @@ func GardenTests(t *testing.T) {
 		// Create new Garden with LightOnTime in the near future, so LightDelay will assume the light is currently off,
 		// meaning adhoc action is going to be predictably delayed
 		maxZones := uint(1)
-		startTime := time.Now().In(time.Local).Add(1 * time.Second).Truncate(time.Second)
+		startTime := pkg.NewStartTime(time.Now().In(time.Local).Add(1 * time.Second).Truncate(time.Second))
 		newGarden := &pkg.Garden{
 			Name:        "TestGarden",
 			TopicPrefix: "test",
 			MaxZones:    &maxZones,
 			LightSchedule: &pkg.LightSchedule{
 				Duration:  &pkg.Duration{Duration: 14 * time.Hour},
-				StartTime: startTime.Format(pkg.LightTimeFormat),
+				StartTime: startTime,
 			},
 		}
 
@@ -197,7 +197,7 @@ func GardenTests(t *testing.T) {
 		status, err = makeRequest(http.MethodGet, fmt.Sprintf("/gardens/%s", g.ID.String()), http.NoBody, &getG)
 		assert.NoError(t, err)
 		assert.Equal(t, http.StatusOK, status)
-		assert.Equal(t, startTime.Add(1*time.Second), getG.NextLightAction.Time.Local())
+		assert.Equal(t, startTime.Time.Add(1*time.Second), getG.NextLightAction.Time.Local())
 
 		time.Sleep(3 * time.Second)
 
@@ -209,17 +209,17 @@ func GardenTests(t *testing.T) {
 	})
 	t.Run("ChangeLightScheduleStartTimeResetsLightSchedule", func(t *testing.T) {
 		// Reschedule Light to turn in in 1 second, for 1 second
-		newStartTime := time.Now().Add(1 * time.Second).Truncate(time.Second)
+		newStartTime := pkg.NewStartTime(time.Now().Add(1 * time.Second).Truncate(time.Second))
 		var g server.GardenResponse
 		status, err := makeRequest(http.MethodPatch, "/gardens/"+gardenID, pkg.Garden{
 			LightSchedule: &pkg.LightSchedule{
-				StartTime: newStartTime.Format(pkg.LightTimeFormat),
+				StartTime: newStartTime,
 				Duration:  &pkg.Duration{Duration: time.Second},
 			},
 		}, &g)
 		assert.NoError(t, err)
 		assert.Equal(t, http.StatusOK, status)
-		assert.Equal(t, newStartTime.Format(pkg.LightTimeFormat), g.LightSchedule.StartTime)
+		assert.Equal(t, newStartTime.String(), g.LightSchedule.StartTime.String())
 
 		time.Sleep(100 * time.Millisecond)
 
@@ -228,7 +228,7 @@ func GardenTests(t *testing.T) {
 		status, err = makeRequest(http.MethodGet, "/gardens/"+gardenID, nil, &g2)
 		assert.NoError(t, err)
 		assert.Equal(t, http.StatusOK, status)
-		assert.Equal(t, newStartTime, g2.NextLightAction.Time.Truncate(time.Second).Local())
+		assert.Equal(t, newStartTime.String(), pkg.NewStartTime(g2.NextLightAction.Time.Local()).String())
 		assert.Equal(t, pkg.LightStateOn, g2.NextLightAction.State)
 
 		time.Sleep(2 * time.Second)
@@ -287,7 +287,7 @@ func CreateWaterScheduleTest(t *testing.T) string {
 		status, err := makeRequest(http.MethodPost, "/water_schedules", `{
 			"duration": "10s",
 			"interval": "24h",
-			"start_time": "2022-04-23T08:00:00-07:00"
+			"start_time": "08:00:00-07:00"
 		}`, &ws)
 		assert.NoError(t, err)
 
@@ -371,12 +371,12 @@ func ZoneTests(t *testing.T) {
 		newStartTime := time.Now().Add(2 * time.Second).Truncate(time.Second)
 		var ws server.WaterScheduleResponse
 		status, err := makeRequest(http.MethodPatch, "/water_schedules/"+waterScheduleID, pkg.WaterSchedule{
-			StartTime: &newStartTime,
+			StartTime: pkg.NewStartTime(newStartTime),
 			Duration:  &pkg.Duration{Duration: time.Second},
 		}, &ws)
 		assert.NoError(t, err)
 		assert.Equal(t, http.StatusOK, status)
-		assert.Equal(t, newStartTime, ws.WaterSchedule.StartTime.Local())
+		assert.Equal(t, pkg.NewStartTime(newStartTime).String(), ws.WaterSchedule.StartTime.String())
 
 		time.Sleep(100 * time.Millisecond)
 
@@ -422,12 +422,12 @@ func WaterScheduleTests(t *testing.T) {
 		newStartTime := time.Now().Add(2 * time.Second).Truncate(time.Second)
 		var ws server.WaterScheduleResponse
 		status, err := makeRequest(http.MethodPatch, "/water_schedules/"+waterScheduleID, pkg.WaterSchedule{
-			StartTime: &newStartTime,
+			StartTime: pkg.NewStartTime(newStartTime),
 			Duration:  &pkg.Duration{Duration: time.Second},
 		}, &ws)
 		assert.NoError(t, err)
 		assert.Equal(t, http.StatusOK, status)
-		assert.Equal(t, newStartTime, ws.WaterSchedule.StartTime.Local())
+		assert.Equal(t, pkg.NewStartTime(newStartTime).String(), ws.WaterSchedule.StartTime.String())
 
 		time.Sleep(100 * time.Millisecond)
 

garden-app/pkg/duration_test.go

@@ -2,9 +2,11 @@ package pkg
 
 import (
 	"encoding/json"
+	"net/url"
 	"testing"
 	"time"
 
+	"github.com/ajg/form"
 	"github.com/stretchr/testify/assert"
 	"gopkg.in/yaml.v3"
 )
@@ -158,3 +160,37 @@ func TestDurationYAMLMarshal(t *testing.T) {
 		assert.Equal(t, "cron:*/5 * * * 1\n", string(result))
 	})
 }
+
+func TestDurationUnmarshalText(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    url.Values
+		expected Duration
+	}{
+		{
+			"DurationString",
+			url.Values{
+				"Duration": []string{"1m0s"},
+			},
+			Duration{Duration: 1 * time.Minute},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var result struct {
+				Duration Duration
+			}
+			err := form.DecodeString(&result, tt.input.Encode())
+			assert.NoError(t, err)
+			assert.Equal(t, tt.expected, result.Duration)
+
+			var formResult struct {
+				Duration Duration
+			}
+			err = form.DecodeValues(&formResult, tt.input)
+			assert.NoError(t, err)
+			assert.Equal(t, tt.expected, formResult.Duration)
+		})
+	}
+}

garden-app/pkg/garden.go

@@ -108,7 +108,7 @@ func (g *Garden) Patch(newGarden *Garden) *babyapi.ErrResponse {
 		g.LightSchedule.Patch(newGarden.LightSchedule)
 
 		// If both Duration and StartTime are empty, remove the schedule
-		if newGarden.LightSchedule.Duration == nil && newGarden.LightSchedule.StartTime == "" {
+		if newGarden.LightSchedule.Duration == nil && newGarden.LightSchedule.StartTime == nil {
 			g.LightSchedule = nil
 		}
 	}
@@ -158,28 +158,21 @@ func (g *Garden) Bind(r *http.Request) error {
 		} else if *g.MaxZones == 0 {
 			return errors.New("max_zones must not be 0")
 		}
-		// consider empty LightSchedule as nil
-		if g.LightSchedule != nil && (g.LightSchedule.Duration == nil || g.LightSchedule.Duration.Duration == 0) && g.LightSchedule.StartTime == "" {
-			g.LightSchedule = nil
+		// consider empty LightSchedule as nil for removing from HTML form
+		if g.LightSchedule != nil && (g.LightSchedule.Duration == nil || g.LightSchedule.Duration.Duration == 0) {
+			startTimeEmpty := g.LightSchedule.StartTime == nil || g.LightSchedule.StartTime.Time.IsZero()
+			if startTimeEmpty {
+				g.LightSchedule = nil
+			}
 		}
 		if g.LightSchedule != nil {
 			if g.LightSchedule.Duration == nil {
 				return errors.New("missing required light_schedule.duration field")
 			}
 
-			// Check that Duration is valid Duration
-			if g.LightSchedule.Duration.Duration >= 24*time.Hour {
-				return fmt.Errorf("invalid light_schedule.duration >= 24 hours: %s", g.LightSchedule.Duration)
-			}
-
-			if g.LightSchedule.StartTime == "" {
+			if g.LightSchedule.StartTime == nil {
 				return errors.New("missing required light_schedule.start_time field")
 			}
-			// Check that LightSchedule.StartTime is valid
-			_, err := g.LightSchedule.ParseStartTime()
-			if err != nil {
-				return err
-			}
 		}
 	case http.MethodPatch:
 		illegalRegexp := regexp.MustCompile(`[\$\#\*\>\+\/]`)
@@ -192,20 +185,19 @@ func (g *Garden) Bind(r *http.Request) error {
 		if g.MaxZones != nil && *g.MaxZones == 0 {
 			return errors.New("max_zones must not be 0")
 		}
+	}
 
-		if g.LightSchedule != nil {
-			// Check that Duration is valid Duration
-			if g.LightSchedule.Duration != nil {
-				if g.LightSchedule.Duration.Duration >= 24*time.Hour {
-					return fmt.Errorf("invalid light_schedule.duration >= 24 hours: %s", g.LightSchedule.Duration)
-				}
+	if g.LightSchedule != nil {
+		if g.LightSchedule.StartTime != nil {
+			err = g.LightSchedule.StartTime.Validate()
+			if err != nil {
+				return err
 			}
-			// Check that LightSchedule.StartTime is valid
-			if g.LightSchedule.StartTime != "" {
-				_, err := g.LightSchedule.ParseStartTime()
-				if err != nil {
-					return err
-				}
+		}
+		// Check that Duration is valid Duration
+		if g.LightSchedule.Duration != nil {
+			if g.LightSchedule.Duration.Duration >= 24*time.Hour {
+				return fmt.Errorf("invalid light_schedule.duration >= 24 hours: %s", g.LightSchedule.Duration)
 			}
 		}
 	}

garden-app/pkg/garden_test.go

@@ -118,7 +118,7 @@ func TestGardenPatch(t *testing.T) {
 		{
 			"PatchLightSchedule.StartTime",
 			&Garden{LightSchedule: &LightSchedule{
-				StartTime: "start time",
+				StartTime: NewStartTime(time.Date(0, 1, 1, 15, 4, 0, 0, time.FixedZone("", 0))),
 			}},
 		},
 		{
@@ -159,7 +159,7 @@ func TestGardenPatch(t *testing.T) {
 	t.Run("RemoveLightSchedule", func(t *testing.T) {
 		g := &Garden{
 			LightSchedule: &LightSchedule{
-				StartTime: "START TIME",
+				StartTime: NewStartTime(time.Date(0, 1, 1, 15, 4, 0, 0, time.FixedZone("", 0))),
 				Duration:  &Duration{2 * time.Hour, ""},
 			},
 		}

garden-app/pkg/light_schedule.go

@@ -7,11 +7,6 @@ import (
 	"time"
 )
 
-const (
-	// LightTimeFormat is used to control format of time fields
-	LightTimeFormat = "15:04:05-07:00"
-)
-
 const (
 	// LightStateOff is the value used to turn off a light
 	LightStateOff LightState = iota
@@ -72,7 +67,7 @@ func (l *LightState) unmarshal(data []byte) error {
 // "Time" should be in the format of LightTimeFormat constant ("15:04:05-07:00")
 type LightSchedule struct {
 	Duration    *Duration  `json:"duration" yaml:"duration"`
-	StartTime   string     `json:"start_time" yaml:"start_time"`
+	StartTime   *StartTime `json:"start_time" yaml:"start_time"`
 	AdhocOnTime *time.Time `json:"adhoc_on_time,omitempty" yaml:"adhoc_on_time,omitempty"`
 }
 
@@ -86,19 +81,10 @@ func (ls *LightSchedule) Patch(new *LightSchedule) {
 	if new.Duration != nil {
 		ls.Duration = new.Duration
 	}
-	if new.StartTime != "" {
+	if new.StartTime != nil {
 		ls.StartTime = new.StartTime
 	}
 	if new.AdhocOnTime == nil {
 		ls.AdhocOnTime = nil
 	}
 }
-
-func (ls *LightSchedule) ParseStartTime() (time.Time, error) {
-	startTime, err := time.Parse(LightTimeFormat, ls.StartTime)
-	if err != nil {
-		return time.Time{}, fmt.Errorf("invalid time format for light_schedule.start_time: %s", ls.StartTime)
-	}
-
-	return startTime, nil
-}