From b385088ba1b756de5449da9935dbd467da4a2340 Mon Sep 17 00:00:00 2001 From: Paulin Todev Date: Thu, 19 Sep 2024 14:05:14 +0100 Subject: [PATCH] Rework duration support --- pkg/codegen/model.go | 17 ++++++++ pkg/codegen/utils.go | 16 +------ pkg/generator/schema_generator.go | 14 +++++++ pkg/generator/validator.go | 58 +++++++++++++++++++++++++- pkg/types/duration.go | 34 --------------- tests/data/core/duration/duration.go | 30 ++++++++++++- tests/data/core/duration/duration.json | 11 ++--- tests/generation_test.go | 6 ++- 8 files changed, 128 insertions(+), 58 deletions(-) delete mode 100644 pkg/types/duration.go diff --git a/pkg/codegen/model.go b/pkg/codegen/model.go index 123d9728..19344c6b 100644 --- a/pkg/codegen/model.go +++ b/pkg/codegen/model.go @@ -262,6 +262,23 @@ func (t NamedType) Generate(out *Emitter) { out.Printf("%s", t.Decl.Name) } +// We need special handling when validating durations. +// Having a dedicated type for durations allows the validator to +// differentiate between durations and other types. +type DurationType struct{} + +func (t DurationType) GetName() string { + return "Duration" +} + +func (t DurationType) IsNillable() bool { + return false +} + +func (t DurationType) Generate(out *Emitter) { + out.Printf("time.Duration") +} + type PrimitiveType struct { Type string } diff --git a/pkg/codegen/utils.go b/pkg/codegen/utils.go index ba81835d..1aa54b71 100644 --- a/pkg/codegen/utils.go +++ b/pkg/codegen/utils.go @@ -111,21 +111,7 @@ func PrimitiveTypeFromJSONSchemaType( } case "duration": - t = NamedType{ - Package: &Package{ - QualifiedName: "time", - Imports: []Import{ - { - //TODO: Should we use the new internal Duration from pkg/types? - // Are the Marshal/Unmarshal functions important? - QualifiedName: "time", - }, - }, - }, - Decl: &TypeDecl{ - Name: "Duration", - }, - } + t = DurationType{} default: t = PrimitiveType{"string"} diff --git a/pkg/generator/schema_generator.go b/pkg/generator/schema_generator.go index e89a777f..719ac419 100644 --- a/pkg/generator/schema_generator.go +++ b/pkg/generator/schema_generator.go @@ -267,6 +267,10 @@ func (g *schemaGenerator) generateDeclaredType( func (g *schemaGenerator) addValidatorsToType(validators []validator, decl codegen.TypeDecl) { if len(validators) > 0 { for _, v := range validators { + for _, im := range v.desc().imports { + g.output.file.Package.AddImport(im.qualifiedName, im.alias) + } + if v.desc().hasError { g.output.file.Package.AddImport("fmt", "") @@ -462,6 +466,11 @@ func (g *schemaGenerator) generateType( return ncg, nil } + if dcg, ok := cg.(codegen.DurationType); ok { + g.output.file.Package.AddImport("time", "") + return dcg, nil + } + return cg, nil } } @@ -746,6 +755,11 @@ func (g *schemaGenerator) generateTypeInline( return ncg, nil } + if dcg, ok := cg.(codegen.DurationType); ok { + g.output.file.Package.AddImport("time", "") + return dcg, nil + } + return cg, nil } diff --git a/pkg/generator/validator.go b/pkg/generator/validator.go index 88da64eb..f9f5e32e 100644 --- a/pkg/generator/validator.go +++ b/pkg/generator/validator.go @@ -7,6 +7,7 @@ import ( "github.com/pkg/errors" "github.com/sanity-io/litter" + "github.com/sosodev/duration" "github.com/atombender/go-jsonschema/pkg/codegen" "github.com/atombender/go-jsonschema/pkg/mathutils" @@ -17,10 +18,16 @@ type validator interface { desc() *validatorDesc } +type packageImport struct { + qualifiedName string + alias string +} + type validatorDesc struct { hasError bool beforeJSONUnmarshal bool requiresRawAfter bool + imports []packageImport } var ( @@ -102,7 +109,7 @@ func (v *nullTypeValidator) desc() *validatorDesc { } } -//TODO: Make a durationValidator, so that we can convert the default value from ISO 8601 to time.Duration. +// TODO: Make a durationValidator, so that we can convert the default value from ISO 8601 to time.Duration. // We could use https://github.com/sosodev/duration type defaultValidator struct { @@ -113,6 +120,42 @@ type defaultValidator struct { } func (v *defaultValidator) generate(out *codegen.Emitter) { + _, ok := v.defaultValueType.(codegen.DurationType) + if v.defaultValueType != nil && ok { + defaultDurationISO8601, ok := v.defaultValue.(string) + if !ok { + // TODO: Return an error instead of panicking? + // TODO: Print type name? + panic("duration default value must be a string") + } + if defaultDurationISO8601 == "" { + // TODO: What should we do if the default is an empty string? + // TODO: Return an error instead of panicking? + // TODO: Print type name? + panic("duration default value must not be an empty string") + } + duration, err := duration.Parse(defaultDurationISO8601) + if err != nil { + // TODO: Return an error instead of panicking? + // TODO: Print type name? + panic("could not convert duration from ISO8601 to Go format") + } + defaultValue := "defaultDuration" + goDurationStr := duration.ToTimeDuration().String() + out.Printlnf(`if v, ok := %s["%s"]; !ok || v == nil {`, varNameRawMap, v.jsonName) + out.Indent(1) + out.Printlnf("%s, err := time.ParseDuration(\"%s\")", defaultValue, goDurationStr) + out.Printlnf("if err != nil {") + out.Indent(1) + out.Printlnf("return fmt.Errorf(\"failed to parse the \\\"%s\\\" default value for field %s:%%w }\", err)", goDurationStr, v.jsonName) + out.Indent(-1) + out.Printlnf("}") + out.Printlnf(`%s.%s = %s`, varNamePlainStruct, v.fieldName, defaultValue) + out.Indent(-1) + out.Printlnf("}") + return + } + defaultValue, err := v.tryDumpDefaultSlice(out.MaxLineLength()) if err != nil { // Fallback to sdump in case we couldn't dump it properly. @@ -153,10 +196,23 @@ func (v *defaultValidator) tryDumpDefaultSlice(maxLineLen int32) (string, error) } func (v *defaultValidator) desc() *validatorDesc { + var packages []packageImport + _, ok := v.defaultValueType.(codegen.DurationType) + if v.defaultValueType != nil && ok { + defaultDurationISO8601, ok := v.defaultValue.(string) + if ok && defaultDurationISO8601 != "" { + packages = []packageImport{ + {qualifiedName: "fmt"}, + {qualifiedName: "time"}, + } + } + } + return &validatorDesc{ hasError: false, beforeJSONUnmarshal: false, requiresRawAfter: true, + imports: packages, } } diff --git a/pkg/types/duration.go b/pkg/types/duration.go deleted file mode 100644 index 57b5c4e4..00000000 --- a/pkg/types/duration.go +++ /dev/null @@ -1,34 +0,0 @@ -package types - -import ( - "fmt" - "time" - - "github.com/sosodev/duration" -) - -// TODO: Can we just call this "Duration"? -type SerializableDuration struct { - time.Duration -} - -func (date SerializableDuration) MarshalJSON() ([]byte, error) { - //TODO: Implement this later - return []byte(""), nil -} - -func (date *SerializableDuration) UnmarshalJSON(data []byte) error { - stringifiedData := string(data) - if stringifiedData == "null" { - return nil - } - - d, err := duration.Parse(stringifiedData) - if err != nil { - return fmt.Errorf("unable to parse duration from JSON: %w", err) - } - - date.Duration = d.ToTimeDuration() - - return nil -} diff --git a/tests/data/core/duration/duration.go b/tests/data/core/duration/duration.go index 4e8032a2..b19b4a58 100644 --- a/tests/data/core/duration/duration.go +++ b/tests/data/core/duration/duration.go @@ -3,6 +3,7 @@ package test import "encoding/json" +import "fmt" import "time" type Duration struct { @@ -11,6 +12,31 @@ type Duration struct { } type DurationMyObject struct { - // MyDateTime corresponds to the JSON schema field "myDateTime". - MyDateTime *time.Duration `json:"myDateTime,omitempty" yaml:"myDateTime,omitempty" mapstructure:"myDateTime,omitempty"` + // WithDefault corresponds to the JSON schema field "withDefault". + WithDefault time.Duration `json:"withDefault,omitempty" yaml:"withDefault,omitempty" mapstructure:"withDefault,omitempty"` + + // WithoutDefault corresponds to the JSON schema field "withoutDefault". + WithoutDefault *time.Duration `json:"withoutDefault,omitempty" yaml:"withoutDefault,omitempty" mapstructure:"withoutDefault,omitempty"` +} + +// UnmarshalJSON implements json.Unmarshaler. +func (j *DurationMyObject) UnmarshalJSON(b []byte) error { + var raw map[string]interface{} + if err := json.Unmarshal(b, &raw); err != nil { + return err + } + type Plain DurationMyObject + var plain Plain + if err := json.Unmarshal(b, &plain); err != nil { + return err + } + if v, ok := raw["withDefault"]; !ok || v == nil { + defaultDuration, err := time.ParseDuration("20s") + if err != nil { + return fmt.Errorf("failed to parse the \"20s\" default value for field withDefault:%w }", err) + } + plain.WithDefault = defaultDuration + } + *j = DurationMyObject(plain) + return nil } diff --git a/tests/data/core/duration/duration.json b/tests/data/core/duration/duration.json index b5e402c8..8b5820b4 100644 --- a/tests/data/core/duration/duration.json +++ b/tests/data/core/duration/duration.json @@ -6,14 +6,15 @@ "properties": { "myObject": { "type": "object", - "required": [ - "myDuration" - ], "properties": { - "myDateTime": { + "withDefault": { "type": "string", "format": "duration", - "default": "P200MS" + "default": "PT20S" + }, + "withoutDefault": { + "type": "string", + "format": "duration" } } } diff --git a/tests/generation_test.go b/tests/generation_test.go index be2b9921..3bdb0ceb 100644 --- a/tests/generation_test.go +++ b/tests/generation_test.go @@ -33,6 +33,10 @@ var ( func TestCore(t *testing.T) { t.Parallel() + // TODO: Additional duration tests: + // * A duration with default = "" + // * A schema file with only one duration. + // It shouldn't have a default, so we ca see if "time" is imported. testExamples(t, basicConfig, "./data/core") } @@ -243,7 +247,7 @@ func testExampleFile(t *testing.T, cfg generator.Config, fileName string) { } if diff, ok := diffStrings(t, string(goldenData), string(source)); !ok { - //TODO: Revert this later + // TODO: Revert this later // t.Fatalf("Contents different (left is expected, right is actual):\n%s", *diff) t.Fatalf("Contents different. Actual:\n%s\nDiff:\n%s", string(source), *diff) }