Use pointers for optional fields

Also:
- Refactor how exposed type is converted to proto type
- Remove unnecesary log
- Input validation for job api calls

Signed-off-by: Albert Callarisa <albert@diagrid.io>

Author: acroca

client/jobs.go

@@ -15,11 +15,10 @@ package client
 
 import (
 	"context"
-	"log"
+	"errors"
 	"time"
 
 	"google.golang.org/protobuf/types/known/anypb"
-	"google.golang.org/protobuf/types/known/durationpb"
 
 	commonpb "github.com/dapr/dapr/pkg/proto/common/v1"
 	runtimepb "github.com/dapr/dapr/pkg/proto/runtime/v1"
@@ -35,18 +34,18 @@ type JobFailurePolicyConstant struct {
 }
 
 func (f *JobFailurePolicyConstant) GetPBFailurePolicy() *commonpb.JobFailurePolicy {
-	policy := &commonpb.JobFailurePolicy{
-		Policy: &commonpb.JobFailurePolicy_Constant{
-			Constant: &commonpb.JobFailurePolicyConstant{},
-		},
-	}
+	constantfp := &commonpb.JobFailurePolicyConstant{}
 	if f.maxRetries != nil {
-		policy.Policy.(*commonpb.JobFailurePolicy_Constant).Constant.MaxRetries = f.maxRetries
+		constantfp.MaxRetries = f.maxRetries
 	}
 	if f.interval != nil {
-		policy.Policy.(*commonpb.JobFailurePolicy_Constant).Constant.Interval = &durationpb.Duration{Seconds: int64(f.interval.Seconds())}
+		constantfp.Interval = toProtoDuration(*f.interval)
+	}
+	return &commonpb.JobFailurePolicy{
+		Policy: &commonpb.JobFailurePolicy_Constant{
+			Constant: constantfp,
+		},
 	}
-	return policy
 }
 
 type JobFailurePolicyDrop struct {
@@ -73,36 +72,30 @@ func NewFailurePolicyDrop() FailurePolicy {
 
 type Job struct {
 	Name          string
-	Schedule      string // Optional
-	Repeats       uint32 // Optional
-	DueTime       string // Optional
-	TTL           string // Optional
+	Schedule      *string
+	Repeats       *uint32
+	DueTime       *string
+	TTL           *string
 	Data          *anypb.Any
 	FailurePolicy FailurePolicy
 }
 
 // ScheduleJobAlpha1 raises and schedules a job.
 func (c *GRPCClient) ScheduleJobAlpha1(ctx context.Context, job *Job) error {
-	// TODO: Assert job fields are defined: Name, Data
-	jobRequest := &runtimepb.Job{
-		Name: job.Name,
-		Data: job.Data,
-	}
-
-	if job.Schedule != "" {
-		jobRequest.Schedule = &job.Schedule
+	if job.Name == "" {
+		return errors.New("job name is required")
 	}
-
-	if job.Repeats != 0 {
-		jobRequest.Repeats = &job.Repeats
+	if job.Data == nil {
+		return errors.New("job data is required")
 	}
 
-	if job.DueTime != "" {
-		jobRequest.DueTime = &job.DueTime
-	}
-
-	if job.TTL != "" {
-		jobRequest.Ttl = &job.TTL
+	jobRequest := &runtimepb.Job{
+		Name:     job.Name,
+		Data:     job.Data,
+		Schedule: job.Schedule,
+		Repeats:  job.Repeats,
+		DueTime:  job.DueTime,
+		Ttl:      job.TTL,
 	}
 
 	if job.FailurePolicy != nil {
@@ -116,11 +109,13 @@ func (c *GRPCClient) ScheduleJobAlpha1(ctx context.Context, job *Job) error {
 
 // GetJobAlpha1 retrieves a scheduled job.
 func (c *GRPCClient) GetJobAlpha1(ctx context.Context, name string) (*Job, error) {
-	// TODO: Name validation
+	if name == "" {
+		return nil, errors.New("job name is required")
+	}
+
 	resp, err := c.protoClient.GetJobAlpha1(ctx, &runtimepb.GetJobRequest{
 		Name: name,
 	})
-	log.Println(resp)
 	if err != nil {
 		return nil, err
 	}
@@ -139,18 +134,21 @@ func (c *GRPCClient) GetJobAlpha1(ctx context.Context, name string) (*Job, error
 
 	return &Job{
 		Name:          resp.GetJob().GetName(),
-		Schedule:      resp.GetJob().GetSchedule(),
-		Repeats:       resp.GetJob().GetRepeats(),
-		DueTime:       resp.GetJob().GetDueTime(),
-		TTL:           resp.GetJob().GetTtl(),
+		Schedule:      resp.GetJob().Schedule,
+		Repeats:       resp.GetJob().Repeats,
+		DueTime:       resp.GetJob().DueTime,
+		TTL:           resp.GetJob().Ttl,
 		Data:          resp.GetJob().GetData(),
 		FailurePolicy: failurePolicy,
 	}, nil
 }
 
 // DeleteJobAlpha1 deletes a scheduled job.
 func (c *GRPCClient) DeleteJobAlpha1(ctx context.Context, name string) error {
-	// TODO: Name validation
+	if name == "" {
+		return errors.New("job name is required")
+	}
+
 	_, err := c.protoClient.DeleteJobAlpha1(ctx, &runtimepb.DeleteJobRequest{
 		Name: name,
 	})

client/jobs_test.go

@@ -26,9 +26,10 @@ func TestSchedulingAlpha1(t *testing.T) {
 	ctx := t.Context()
 
 	t.Run("schedule job - valid", func(t *testing.T) {
+		schedule := "test"
 		err := testClient.ScheduleJobAlpha1(ctx, &Job{
 			Name:          "test",
-			Schedule:      "test",
+			Schedule:      &schedule,
 			Data:          &anypb.Any{},
 			FailurePolicy: NewFailurePolicyConstant(nil, nil),
 		})
@@ -40,12 +41,17 @@ func TestSchedulingAlpha1(t *testing.T) {
 		maxRetries := uint32(4)
 		interval := time.Second * 10
 
+		schedule := "@every 10s"
+		repeats := uint32(4)
+		dueTime := "10s"
+		ttl := "10s"
+
 		expected := &Job{
 			Name:          "name",
-			Schedule:      "@every 10s",
-			Repeats:       4,
-			DueTime:       "10s",
-			TTL:           "10s",
+			Schedule:      &schedule,
+			Repeats:       &repeats,
+			DueTime:       &dueTime,
+			TTL:           &ttl,
 			Data:          nil,
 			FailurePolicy: NewFailurePolicyConstant(&maxRetries, &interval),
 		}

client/state.go

@@ -17,10 +17,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"math"
-	"time"
-
-	"google.golang.org/protobuf/types/known/durationpb"
 
 	v1 "github.com/dapr/dapr/pkg/proto/common/v1"
 	pb "github.com/dapr/dapr/pkg/proto/runtime/v1"
@@ -249,22 +245,6 @@ func copyStateOptionDefault() *StateOptions {
 	}
 }
 
-func toProtoDuration(d time.Duration) *durationpb.Duration {
-	nanos := d.Nanoseconds()
-	secs := nanos / 1e9
-	nanos -= secs * 1e9
-
-	// conversion check - gosec ignored below for conversion
-	if nanos <= int64(math.MinInt32) && nanos >= int64(math.MaxInt32) {
-		panic("integer overflow converting duration to proto")
-	}
-
-	return &durationpb.Duration{
-		Seconds: secs,
-		Nanos:   int32(nanos), //nolint:gosec
-	}
-}
-
 // ExecuteStateTransaction provides way to execute multiple operations on a specified store.
 func (c *GRPCClient) ExecuteStateTransaction(ctx context.Context, storeName string, meta map[string]string, ops []*StateOperation) error {
 	if storeName == "" {

client/utils.go

@@ -13,7 +13,13 @@ limitations under the License.
 
 package client
 
-import "encoding/json"
+import (
+	"encoding/json"
+	"math"
+	"time"
+
+	"google.golang.org/protobuf/types/known/durationpb"
+)
 
 // isCloudEvent returns true if the event is a CloudEvent.
 // An event is a CloudEvent if it `id`, `source`, `specversion` and `type` fields.
@@ -30,3 +36,20 @@ func isCloudEvent(event []byte) bool {
 	}
 	return ce.ID != "" && ce.Source != "" && ce.SpecVersion != "" && ce.Type != ""
 }
+
+// toProtoDuration converts a time.Duration to a protobuf duration.
+func toProtoDuration(d time.Duration) *durationpb.Duration {
+	nanos := d.Nanoseconds()
+	secs := nanos / 1e9
+	nanos -= secs * 1e9
+
+	// conversion check - gosec ignored below for conversion
+	if nanos <= int64(math.MinInt32) && nanos >= int64(math.MaxInt32) {
+		panic("integer overflow converting duration to proto")
+	}
+
+	return &durationpb.Duration{
+		Seconds: secs,
+		Nanos:   int32(nanos), //nolint:gosec
+	}
+}