From 9e3d7bf4ac0d7d4520654b0a01b1c8fe96d797bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Gomez?= Date: Wed, 4 Oct 2023 17:39:54 +0200 Subject: [PATCH] Compose panel Options and FieldConfig composite types into a unified panel builder --- internal/jennies/golang/jennies.go | 16 ++- internal/jennies/typescript/jennies.go | 16 ++- internal/veneers/builder/actions.go | 116 ----------------- internal/veneers/builder/rules.go | 172 +++++++++++++++++++++++-- internal/veneers/builder/selectors.go | 11 +- internal/veneers/rewrite.go | 31 +++-- internal/veneers/rewrite_test.go | 2 +- internal/veneers/veneers.go | 18 ++- 8 files changed, 223 insertions(+), 159 deletions(-) delete mode 100644 internal/veneers/builder/actions.go diff --git a/internal/jennies/golang/jennies.go b/internal/jennies/golang/jennies.go index 12f27a2ab..f2237a853 100644 --- a/internal/jennies/golang/jennies.go +++ b/internal/jennies/golang/jennies.go @@ -21,14 +21,26 @@ func Jennies() *codejen.JennyList[[]*ast.Schema] { codejen.AdaptOneToMany[[]ast.Builder, []*ast.Schema]( &Builder{}, func(schemas []*ast.Schema) []ast.Builder { + var err error + generator := &ast.BuilderGenerator{} builders := generator.FromAST(schemas) // apply common veneers - builders = veneers.Common().ApplyTo(builders) + builders, err = veneers.Common().ApplyTo(builders) + if err != nil { + // FIXME: codejen.AdaptOneToMany() doesn't let us return an error + panic(err) + } // apply Go-specific veneers - return Veneers().ApplyTo(builders) + builders, err = Veneers().ApplyTo(builders) + if err != nil { + // FIXME: codejen.AdaptOneToMany() doesn't let us return an error + panic(err) + } + + return builders }, ), ) diff --git a/internal/jennies/typescript/jennies.go b/internal/jennies/typescript/jennies.go index aaf01f42c..ff6c0fd7a 100644 --- a/internal/jennies/typescript/jennies.go +++ b/internal/jennies/typescript/jennies.go @@ -24,14 +24,26 @@ func Jennies() *codejen.JennyList[[]*ast.Schema] { codejen.AdaptOneToMany[[]ast.Builder, []*ast.Schema]( &Builder{}, func(schemas []*ast.Schema) []ast.Builder { + var err error + generator := &ast.BuilderGenerator{} builders := generator.FromAST(schemas) // apply common veneers - builders = veneers.Common().ApplyTo(builders) + builders, err = veneers.Common().ApplyTo(builders) + if err != nil { + // FIXME: codejen.AdaptOneToMany() doesn't let us return an error + panic(err) + } // apply TS-specific veneers - return Veneers().ApplyTo(builders) + builders, err = Veneers().ApplyTo(builders) + if err != nil { + // FIXME: codejen.AdaptOneToMany() doesn't let us return an error + panic(err) + } + + return builders }, ), ) diff --git a/internal/veneers/builder/actions.go b/internal/veneers/builder/actions.go deleted file mode 100644 index 6bed10e29..000000000 --- a/internal/veneers/builder/actions.go +++ /dev/null @@ -1,116 +0,0 @@ -package builder - -import ( - "strings" - - "github.com/grafana/cog/internal/ast" - "github.com/grafana/cog/internal/tools" -) - -type RewriteAction func(builders ast.Builders, builder ast.Builder) ast.Builder - -func OmitAction() RewriteAction { - return func(builders ast.Builders, _ ast.Builder) ast.Builder { - return ast.Builder{} - } -} - -func MergeIntoAction(sourceBuilderName string, underPath string, excludeOptions []string) RewriteAction { - return func(builders ast.Builders, destinationBuilder ast.Builder) ast.Builder { - sourcePkg, sourceBuilderNameWithoutPkg, found := strings.Cut(sourceBuilderName, ".") - if !found { - sourcePkg = destinationBuilder.Package - sourceBuilderNameWithoutPkg = sourceBuilderName - } - - sourceBuilder, found := builders.LocateByObject(sourcePkg, sourceBuilderNameWithoutPkg) - if !found { - return destinationBuilder - } - - newBuilder := destinationBuilder - - // TODO: initializations - - for _, opt := range sourceBuilder.Options { - if tools.ItemInList(opt.Name, excludeOptions) { - continue - } - - // TODO: assignment paths - newOpt := opt - newOpt.Assignments = nil - - for _, assignment := range opt.Assignments { - newAssignment := assignment - // @FIXME: this only works if no part of the `underPath` path can be nil - newAssignment.Path = underPath + "." + assignment.Path - - newOpt.Assignments = append(newOpt.Assignments, newAssignment) - } - - newBuilder.Options = append(newBuilder.Options, newOpt) - } - - return newBuilder - } -} - -func ComposeDashboardPanelAction(panelBuilderName string) RewriteAction { - return func(builders ast.Builders, destinationBuilder ast.Builder) ast.Builder { - panelBuilderPkg, panelBuilderNameWithoutPkg, found := strings.Cut(panelBuilderName, ".") - if !found { - panelBuilderPkg = destinationBuilder.Package - panelBuilderNameWithoutPkg = panelBuilderName - } - - panelBuilder, found := builders.LocateByObject(panelBuilderPkg, panelBuilderNameWithoutPkg) - if !found { - // TODO: we failed here, an error is the correct action. - return destinationBuilder - } - - // we're more building a new thing than updating any of the panel or composable builder, - // so let's create a completely fresh instance - newBuilder := ast.Builder{ - Schema: panelBuilder.Schema, - For: panelBuilder.For, - RootPackage: destinationBuilder.RootPackage, - Package: destinationBuilder.Package, - } - - // re-add panel-related options - for _, panelOpt := range panelBuilder.Options { - // this value is a constant that depends on the plugin being composed into a panel - if panelOpt.Name == "type" { - continue - } - - newBuilder.Options = append(newBuilder.Options, panelOpt) - } - - // re-add plugin-specific options - for _, pluginOpt := range destinationBuilder.Options { - newOpt := pluginOpt - newOpt.Assignments = nil - - // re-root assignments - for _, assignment := range pluginOpt.Assignments { - newAssignment := assignment - newAssignment.Path = "options." + newAssignment.Path - - newOpt.Assignments = append(newOpt.Assignments, newAssignment) - } - - newBuilder.Options = append(newBuilder.Options, newOpt) - } - - newBuilder.Initializations = append(newBuilder.Initializations, ast.Assignment{ - Path: "type", - ValueType: ast.String(), - Value: destinationBuilder.Schema.Metadata.Identifier, - }) - - return newBuilder - } -} diff --git a/internal/veneers/builder/rules.go b/internal/veneers/builder/rules.go index 2d96abb89..85c1bdbcb 100644 --- a/internal/veneers/builder/rules.go +++ b/internal/veneers/builder/rules.go @@ -1,27 +1,173 @@ package builder -type RewriteRule struct { - Selector Selector - Action RewriteAction +import ( + "fmt" + "strings" + + "github.com/grafana/cog/internal/ast" + "github.com/grafana/cog/internal/tools" +) + +type RewriteRule func(builders ast.Builders) (ast.Builders, error) + +func mapToSelected(selector Selector, mapFunc func(builders ast.Builders, builder ast.Builder) (ast.Builder, error)) RewriteRule { + return func(builders ast.Builders) (ast.Builders, error) { + for i, b := range builders { + if !selector(b) { + continue + } + + newBuilder, err := mapFunc(builders, b) + if err != nil { + return nil, err + } + + builders[i] = newBuilder + } + + return builders, nil + } +} + +func mergeOptions(fromBuilder ast.Builder, intoBuilder ast.Builder, underPath string, excludeOptions []string) ast.Builder { + newBuilder := intoBuilder + + for _, opt := range fromBuilder.Options { + if tools.ItemInList(opt.Name, excludeOptions) { + continue + } + + // TODO: assignment paths + newOpt := opt + newOpt.Assignments = nil + + for _, assignment := range opt.Assignments { + newAssignment := assignment + // @FIXME: this only works if no part of the `underPath` path can be nil + newAssignment.Path = underPath + "." + assignment.Path + + newOpt.Assignments = append(newOpt.Assignments, newAssignment) + } + + newBuilder.Options = append(newBuilder.Options, newOpt) + } + + return newBuilder } func Omit(selector Selector) RewriteRule { - return RewriteRule{ - Selector: selector, - Action: OmitAction(), + return func(builders ast.Builders) (ast.Builders, error) { + filteredBuilders := make([]ast.Builder, 0, len(builders)) + + for _, builder := range builders { + if selector(builder) { + continue + } + + filteredBuilders = append(filteredBuilders, builder) + } + + return filteredBuilders, nil } } func MergeInto(selector Selector, sourceBuilderName string, underPath string, excludeOptions []string) RewriteRule { - return RewriteRule{ - Selector: selector, - Action: MergeIntoAction(sourceBuilderName, underPath, excludeOptions), + return mapToSelected(selector, func(builders ast.Builders, destinationBuilder ast.Builder) (ast.Builder, error) { + sourcePkg, sourceBuilderNameWithoutPkg, found := strings.Cut(sourceBuilderName, ".") + if !found { + return destinationBuilder, fmt.Errorf("sourceBuilderName '%s' is incorrect: no package found", sourceBuilderName) + } + + sourceBuilder, found := builders.LocateByObject(sourcePkg, sourceBuilderNameWithoutPkg) + if !found { + return destinationBuilder, fmt.Errorf("source builder '%s.%s' not found", sourcePkg, sourceBuilderNameWithoutPkg) + } + + // TODO: initializations + newBuilder := mergeOptions(sourceBuilder, destinationBuilder, underPath, excludeOptions) + + return newBuilder, nil + }) +} + +func composePanelType(panelType string, panelBuilder ast.Builder, composableBuilders ast.Builders) ast.Builder { + newBuilder := ast.Builder{ + Schema: panelBuilder.Schema, + For: panelBuilder.For, + RootPackage: panelType, + Package: "panel", + } + + newBuilder.Initializations = append(newBuilder.Initializations, ast.Assignment{ + Path: "type", + ValueType: ast.String(), + Value: panelType, + }) + + // re-add panel-related options + for _, panelOpt := range panelBuilder.Options { + // this value is a constant that depends on the plugin being composed into a panel + if panelOpt.Name == "type" { + continue + } + + newBuilder.Options = append(newBuilder.Options, panelOpt) + } + + for _, composableBuilder := range composableBuilders { + if composableBuilder.For.Name == "Options" { + newBuilder = mergeOptions(composableBuilder, newBuilder, "options", nil) + continue + } + + if composableBuilder.For.Name == "FieldConfig" { + newBuilder = mergeOptions(composableBuilder, newBuilder, "fieldConfig.defaults.custom", nil) + continue + } + + panic("unexpected composable type " + composableBuilder.For.Name) } + + return newBuilder } -func ComposeDashboardPanel(selector Selector, sourceBuilderName string) RewriteRule { - return RewriteRule{ - Selector: selector, - Action: ComposeDashboardPanelAction(sourceBuilderName), +func ComposeDashboardPanel(selector Selector, panelBuilderName string) RewriteRule { + return func(builders ast.Builders) (ast.Builders, error) { + panelBuilderPkg, panelBuilderNameWithoutPkg, found := strings.Cut(panelBuilderName, ".") + if !found { + return nil, fmt.Errorf("panelBuilderName '%s' is incorrect: no package found", panelBuilderPkg) + } + + panelBuilder, found := builders.LocateByObject(panelBuilderPkg, panelBuilderNameWithoutPkg) + if !found { + return nil, fmt.Errorf("panel builder '%s' not found", panelBuilderName) + } + + // - add to newBuilders all the builders that are not composable (ie: don't comply to the selector) + // - build a map of composable builders, indexed by panel type + // - aggregate the composable builders into a new, composed panel builder + // - add the new composed panel builders to newBuilders + + newBuilders := make([]ast.Builder, 0, len(builders)) + composableBuilders := make(map[string]ast.Builders) + + for _, builder := range builders { + // the builder is for a composable type + if selector(builder) { + panelType := builder.Schema.Metadata.Identifier + composableBuilders[panelType] = append(composableBuilders[panelType], builder) + continue + } + + newBuilders = append(newBuilders, builder) + } + + for panelType, buildersForType := range composableBuilders { + composedBuilder := composePanelType(panelType, panelBuilder, buildersForType) + + newBuilders = append(newBuilders, composedBuilder) + } + + return newBuilders, nil } } diff --git a/internal/veneers/builder/selectors.go b/internal/veneers/builder/selectors.go index 4eb11bbd5..fbb6f4d37 100644 --- a/internal/veneers/builder/selectors.go +++ b/internal/veneers/builder/selectors.go @@ -1,14 +1,21 @@ package builder import ( + "strings" + "github.com/grafana/cog/internal/ast" ) type Selector func(builder ast.Builder) bool -func ByName(objectName string) Selector { +func ByObjectName(objectName string) Selector { return func(builder ast.Builder) bool { - return builder.For.Name == objectName + objectPkg, objectNameWithoutPkg, found := strings.Cut(objectName, ".") + if !found { + return builder.For.Name == objectName + } + + return builder.For.SelfRef.ReferredPkg == objectPkg && builder.For.SelfRef.ReferredType == objectNameWithoutPkg } } diff --git a/internal/veneers/rewrite.go b/internal/veneers/rewrite.go index 5a526c6de..0610debdc 100644 --- a/internal/veneers/rewrite.go +++ b/internal/veneers/rewrite.go @@ -18,34 +18,33 @@ func NewRewrite(builderRules []builder.RewriteRule, optionRules []option.Rewrite } } -func (engine *Rewriter) ApplyTo(builders []ast.Builder) []ast.Builder { +func (engine *Rewriter) ApplyTo(builders []ast.Builder) ([]ast.Builder, error) { + var err error // TODO: should we deepCopy the builders instead? newBuilders := make([]ast.Builder, 0, len(builders)) newBuilders = append(newBuilders, builders...) - newBuilders = engine.applyBuilderRules(newBuilders) + newBuilders, err = engine.applyBuilderRules(newBuilders) + if err != nil { + return nil, err + } + newBuilders = engine.applyOptionRules(newBuilders) - return newBuilders + return newBuilders, nil } -func (engine *Rewriter) applyBuilderRules(builders []ast.Builder) []ast.Builder { - for _, rule := range engine.builderRules { - for i, b := range builders { - // this builder is being discarded - if len(b.Options) == 0 { - continue - } +func (engine *Rewriter) applyBuilderRules(builders []ast.Builder) ([]ast.Builder, error) { + var err error - if !rule.Selector(b) { - continue - } - - builders[i] = rule.Action(builders, b) + for _, rule := range engine.builderRules { + builders, err = rule(builders) + if err != nil { + return nil, err } } - return engine.filterDiscardedBuilders(builders) + return builders, nil } func (engine *Rewriter) applyOptionRules(builders []ast.Builder) []ast.Builder { diff --git a/internal/veneers/rewrite_test.go b/internal/veneers/rewrite_test.go index d8a28ca3f..7276755c4 100644 --- a/internal/veneers/rewrite_test.go +++ b/internal/veneers/rewrite_test.go @@ -33,7 +33,7 @@ func testData() []rewriteTestCase { description: "omit an entire builder", inputBuilders: ast.Builders{dashboardBuilder(), panelBuilder()}, builderRules: []builder.RewriteRule{ - builder.Omit(builder.ByName("Dashboard")), + builder.Omit(builder.ByObjectName("Dashboard")), }, optionRules: nil, outputBuilders: ast.Builders{panelBuilder()}, diff --git a/internal/veneers/veneers.go b/internal/veneers/veneers.go index 8d994a647..a804a0092 100644 --- a/internal/veneers/veneers.go +++ b/internal/veneers/veneers.go @@ -8,18 +8,22 @@ import ( func Common() *Rewriter { return NewRewrite( []builder.RewriteRule{ + /******************************************** + * Dashboards + ********************************************/ + // We don't want these builders at all - builder.Omit(builder.ByName("GridPos")), - builder.Omit(builder.ByName("DataSourceRef")), - builder.Omit(builder.ByName("LibraryPanelRef")), + builder.Omit(builder.ByObjectName("dashboard.GridPos")), + builder.Omit(builder.ByObjectName("dashboard.DataSourceRef")), + builder.Omit(builder.ByObjectName("dashboard.LibraryPanelRef")), // No need for builders for structs generated from a disjunction builder.Omit(builder.StructGeneratedFromDisjunction()), // rearrange things a bit builder.MergeInto( - builder.ByName("Panel"), - "FieldConfig", + builder.ByObjectName("dashboard.Panel"), + "dashboard.FieldConfig", "fieldConfig.defaults", []string{ @@ -38,8 +42,8 @@ func Common() *Rewriter { ), // remove builders that were previously merged into something else - builder.Omit(builder.ByName("FieldConfig")), - builder.Omit(builder.ByName("FieldConfigSource")), + builder.Omit(builder.ByObjectName("dashboard.FieldConfig")), + builder.Omit(builder.ByObjectName("dashboard.FieldConfigSource")), }, []option.RewriteRule{