diff --git a/repository/codegen.go b/repository/codegen.go index 137ac8c3..11b9135b 100644 --- a/repository/codegen.go +++ b/repository/codegen.go @@ -13,6 +13,7 @@ import ( "github.com/viant/tagly/format/text" "github.com/viant/toolbox/data" "github.com/viant/xreflect" + "go/format" "path" "reflect" "strconv" @@ -190,14 +191,18 @@ func (c *Component) GenerateOutputCode(ctx context.Context, withDefineComponent, if withEmbed { embedderCode := fmt.Sprintf(` - func (i *%vInput) EmbedFS() *embed.FS { - return &%vFS - }`, componentName, componentName) +func (i *%vInput) EmbedFS() *embed.FS { + return &%vFS +} +`, componentName, componentName) builder.WriteString(embedderCode) } result := builder.String() result = c.View.Resource().ReverseSubstitutes(result) + if formatted, err := format.Source([]byte(result)); err == nil { + result = string(formatted) + } return result } diff --git a/repository/codegen_embedfs_test.go b/repository/codegen_embedfs_test.go new file mode 100644 index 00000000..ccd5efc7 --- /dev/null +++ b/repository/codegen_embedfs_test.go @@ -0,0 +1,96 @@ +package repository + +import ( + "context" + "go/format" + "reflect" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/viant/datly/repository/contract" + "github.com/viant/datly/view" + "github.com/viant/datly/view/state" + "github.com/viant/xreflect" +) + +func newEmbedFSTestComponent(t *testing.T) *Component { + t.Helper() + + resource := view.EmptyResource() + rootView := view.NewView("active_advertiser", "ACTIVE_ADVERTISER") + rootView.Connector = &view.Connector{Connection: view.Connection{DBConfig: view.DBConfig{Name: "ci_ads"}}} + rootView.Template = &view.Template{Source: "SELECT ID FROM ACTIVE_ADVERTISER"} + rootView.Schema = state.NewSchema(reflect.TypeOf([]*struct { + Id *int `sqlx:"ID"` + }{})) + resource.Types = []*view.TypeDefinition{ + {Name: "ActiveAdvertiserView", Package: "universalpixel", DataType: `struct{Id *int ` + "`sqlx:\"ID\"`" + `;}`}, + } + require.NoError(t, resource.TypeRegistry().Register("ActiveAdvertiserView", + xreflect.WithPackage("universalpixel"), + xreflect.WithReflectType(reflect.TypeOf(struct { + Id *int `sqlx:"ID"` + }{})))) + rootView.SetResource(resource) + resource.AddViews(rootView) + + inputType, err := state.NewType(state.WithParameters(state.Parameters{ + &state.Parameter{Name: "AdvertiserId", In: state.NewQueryLocation("advertiserId"), Schema: state.NewSchema(reflect.TypeOf(0))}, + }), state.WithResource(&reportTestResource{})) + require.NoError(t, err) + inputType.Name = "ActiveAdvertiserInput" + inputType.Package = "universalpixel" + + outputType, err := state.NewType(state.WithParameters(state.Parameters{ + &state.Parameter{Name: "Data", In: state.NewOutputLocation("view"), Schema: &state.Schema{Name: "ActiveAdvertiserView", Package: "universalpixel", Cardinality: state.Many}}, + }), state.WithResource(rootView.Resource())) + require.NoError(t, err) + outputType.Name = "ActiveAdvertiserOutput" + outputType.Package = "universalpixel" + + return &Component{ + Path: contract.Path{Method: "GET", URI: "/v1/api/platform/universalpixel/activeadvertiser"}, + Meta: contract.Meta{Name: "ActiveAdvertiser"}, + View: rootView, + Contract: contract.Contract{ + Input: contract.Input{Type: *inputType}, + Output: contract.Output{Type: *outputType}, + }, + } +} + +// The generated EmbedFS accessor used to be emitted from a raw string literal +// indented by one tab and without a trailing newline, so every generated reader +// was unformatted Go. Consumers that ran gofmt saw the file churn back on the +// next `datly gen`. +func TestGenerateOutputCode_EmbedFSAccessorIsGoFormatted(t *testing.T) { + component := newEmbedFSTestComponent(t) + + code := component.GenerateOutputCode(context.Background(), false, true, map[string]string{}) + + assert.Contains(t, code, "\nfunc (i *ActiveAdvertiserInput) EmbedFS() *embed.FS {\n\treturn &ActiveAdvertiserFS\n}\n", + "EmbedFS accessor must be emitted at column 0 with a tab-indented body") + assert.NotContains(t, code, "\n\tfunc (", "no top-level func may be indented") + assert.True(t, strings.HasSuffix(code, "\n"), "generated file must end with a newline") +} + +func TestGenerateOutputCode_IsGofmtStable(t *testing.T) { + component := newEmbedFSTestComponent(t) + + code := component.GenerateOutputCode(context.Background(), false, true, map[string]string{}) + + formatted, err := format.Source([]byte(code)) + require.NoError(t, err, "generated code must parse") + assert.Equal(t, string(formatted), code, "generated code must already be gofmt-clean") +} + +// Regenerating an unchanged component must not produce a different file, or +// consumers get spurious diffs on every codegen run. +func TestGenerateOutputCode_IsDeterministic(t *testing.T) { + first := newEmbedFSTestComponent(t).GenerateOutputCode(context.Background(), false, true, map[string]string{}) + second := newEmbedFSTestComponent(t).GenerateOutputCode(context.Background(), false, true, map[string]string{}) + + assert.Equal(t, first, second) +} diff --git a/repository/path/service.go b/repository/path/service.go index 44118dbb..83e735f3 100644 --- a/repository/path/service.go +++ b/repository/path/service.go @@ -13,6 +13,7 @@ import ( "github.com/viant/datly/repository/version" "gopkg.in/yaml.v3" "path" + "sort" "strings" "sync" "time" @@ -151,6 +152,7 @@ func (s *Service) createPathFiles(ctx context.Context) error { if err != nil { return err } + sortByURL(candidates) rootPath := url.Path(s.URL) for _, candidate := range candidates { if candidate.IsDir() { @@ -187,6 +189,18 @@ func (s *Service) createPathFiles(ctx context.Context) error { return nil } +// sortByURL gives paths.yaml a stable entry order. The recursive listing that +// feeds it reflects directory enumeration order, which differs between +// filesystems and shifts whenever route files are rewritten - so without this +// the same repository yields a different paths.yaml on every machine, and any +// partial regeneration reshuffles thousands of lines. Route lookup itself is +// order independent: the matcher builds a trie and prefers exact matches. +func sortByURL(candidates []storage.Object) { + sort.SliceStable(candidates, func(i, j int) bool { + return candidates[i].URL() < candidates[j].URL() + }) +} + func (s *Service) buildPaths(ctx context.Context, candidate storage.Object, rootPath string) (*Item, error) { data, err := s.fs.Download(ctx, candidate) if err != nil { diff --git a/repository/path/sort_test.go b/repository/path/sort_test.go new file mode 100644 index 00000000..03ee61cf --- /dev/null +++ b/repository/path/sort_test.go @@ -0,0 +1,94 @@ +package path + +import ( + spath "path" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/viant/afs/file" + "github.com/viant/afs/object" + "github.com/viant/afs/storage" +) + +func objectURLs(candidates []storage.Object) []string { + var result []string + for _, candidate := range candidates { + result = append(result, candidate.URL()) + } + return result +} + +func newObject(URL string) storage.Object { + name := spath.Base(URL) + return object.New(URL, file.NewInfo(name, 0, file.DefaultFileOsMode, time.Now(), false), nil) +} + +func newObjects(URLs ...string) []storage.Object { + var result []storage.Object + for _, URL := range URLs { + result = append(result, newObject(URL)) + } + return result +} + +// The recursive listing behind paths.yaml reflects directory enumeration order, +// so without an explicit sort the same repository produces a different +// paths.yaml on every machine and any partial regeneration reshuffles it. +func TestSortByURL(t *testing.T) { + testCases := []struct { + description string + urls []string + expect []string + }{ + { + description: "enumeration order is normalised to lexical order", + urls: []string{ + "file:///repo/routes/system/session/session.yaml", + "file:///repo/routes/mdp/adorder/forecast.yaml", + "file:///repo/routes/platform/agency/agency.yaml", + }, + expect: []string{ + "file:///repo/routes/mdp/adorder/forecast.yaml", + "file:///repo/routes/platform/agency/agency.yaml", + "file:///repo/routes/system/session/session.yaml", + }, + }, + { + description: "already sorted stays sorted", + urls: []string{"file:///repo/routes/a.yaml", "file:///repo/routes/b.yaml"}, + expect: []string{"file:///repo/routes/a.yaml", "file:///repo/routes/b.yaml"}, + }, + { + description: "single entry", + urls: []string{"file:///repo/routes/only.yaml"}, + expect: []string{"file:///repo/routes/only.yaml"}, + }, + { + description: "empty listing is a no-op", + urls: nil, + expect: nil, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.description, func(t *testing.T) { + candidates := newObjects(testCase.urls...) + sortByURL(candidates) + assert.Equal(t, testCase.expect, objectURLs(candidates)) + }) + } +} + +// Sorting must be idempotent, otherwise repeated regeneration still churns. +func TestSortByURL_Idempotent(t *testing.T) { + candidates := newObjects( + "file:///repo/routes/z.yaml", + "file:///repo/routes/a.yaml", + "file:///repo/routes/m.yaml", + ) + sortByURL(candidates) + first := objectURLs(candidates) + sortByURL(candidates) + assert.Equal(t, first, objectURLs(candidates)) +} diff --git a/view/column/discover.go b/view/column/discover.go index 5c779fd9..7c03b4a7 100644 --- a/view/column/discover.go +++ b/view/column/discover.go @@ -15,6 +15,7 @@ import ( "github.com/viant/sqlx/io" "github.com/viant/sqlx/io/config" "reflect" + "sort" "github.com/viant/sqlx/metadata/sink" "github.com/viant/xreflect" @@ -250,9 +251,27 @@ func readSinkColumns(ctx context.Context, db *sql.DB, table string) ([]sink.Colu if len(columns) == 0 { return nil, vErr } + sortByPosition(columns) return columns, err } +// sortByPosition orders columns by their ordinal position in the table. +// The information_schema queries behind config.Columns carry no ORDER BY, so +// the driver may return columns in any order - which would otherwise leak into +// generated struct field order and produce spurious diffs between machines. +// Columns inferred from a result set carry no position; leaving them stable +// preserves the projection order the query already established. +func sortByPosition(columns []sink.Column) { + for _, column := range columns { + if column.Position == 0 { + return + } + } + sort.SliceStable(columns, func(i, j int) bool { + return columns[i].Position < columns[j].Position + }) +} + func parseQuery(SQL string) (string, string, sqlparser.Columns) { sqlQuery, _ := sqlparser.ParseQuery(SQL) var table string diff --git a/view/column/discover_sort_test.go b/view/column/discover_sort_test.go new file mode 100644 index 00000000..0e9d0798 --- /dev/null +++ b/view/column/discover_sort_test.go @@ -0,0 +1,83 @@ +package column + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/viant/sqlx/metadata/sink" +) + +func names(columns []sink.Column) []string { + var result []string + for _, column := range columns { + result = append(result, column.Name) + } + return result +} + +// config.Columns runs an information_schema query with no ORDER BY, so the +// driver may hand back columns in any order. Without a sort, that order leaks +// into generated struct field order and churns on every regeneration. +func TestSortByPosition(t *testing.T) { + testCases := []struct { + description string + columns []sink.Column + expect []string + }{ + { + description: "alphabetical metadata order is restored to table order", + columns: []sink.Column{ + {Name: "CREATED", Position: 5}, + {Name: "CREATED_USER", Position: 7}, + {Name: "FEE_DOMAIN", Position: 4}, + {Name: "FEE_TYPE_ID", Position: 3}, + {Name: "ID", Position: 1}, + {Name: "NAME", Position: 2}, + {Name: "UPDATED", Position: 6}, + {Name: "UPDATED_USER", Position: 8}, + }, + expect: []string{"ID", "NAME", "FEE_TYPE_ID", "FEE_DOMAIN", "CREATED", "UPDATED", "CREATED_USER", "UPDATED_USER"}, + }, + { + description: "already ordered stays ordered", + columns: []sink.Column{ + {Name: "ID", Position: 1}, + {Name: "NAME", Position: 2}, + }, + expect: []string{"ID", "NAME"}, + }, + { + description: "result-set inferred columns carry no position, so projection order is preserved", + columns: []sink.Column{ + {Name: "TOTAL_SPEND"}, + {Name: "AGENCY_ID"}, + }, + expect: []string{"TOTAL_SPEND", "AGENCY_ID"}, + }, + { + description: "empty input is a no-op", + columns: []sink.Column{}, + expect: nil, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.description, func(t *testing.T) { + sortByPosition(testCase.columns) + assert.Equal(t, testCase.expect, names(testCase.columns)) + }) + } +} + +// Sorting must be idempotent, otherwise repeated regeneration would still churn. +func TestSortByPosition_Idempotent(t *testing.T) { + columns := []sink.Column{ + {Name: "UPDATED", Position: 6}, + {Name: "ID", Position: 1}, + {Name: "NAME", Position: 2}, + } + sortByPosition(columns) + first := names(columns) + sortByPosition(columns) + assert.Equal(t, first, names(columns)) +}