diff --git a/internals/plan/extensions_test.go b/internals/plan/extensions_test.go index 1c02132fe..bf7cbd54b 100644 --- a/internals/plan/extensions_test.go +++ b/internals/plan/extensions_test.go @@ -42,16 +42,15 @@ type planResult struct { } var extensionTests = []struct { - extensions map[string]plan.LayerSectionExtension - layers []*inputLayer - errorString string - combinedPlanResult *planResult - combinedPlanResultYaml string + extensions map[string]plan.LayerSectionExtension + layers []*inputLayer + error string + result *planResult + resultYaml string }{ // Index 0: No Sections { - combinedPlanResultYaml: string(reindent(` - {}`)), + resultYaml: "{}\n", }, // Index 1: Sections with empty YAML { @@ -59,12 +58,11 @@ var extensionTests = []struct { "x-field": &xExtension{}, "y-field": &yExtension{}, }, - combinedPlanResult: &planResult{ + result: &planResult{ x: &xSection{}, y: &ySection{}, }, - combinedPlanResultYaml: string(reindent(` - {}`)), + resultYaml: "{}\n", }, // Index 2: Load file layers invalid section { @@ -73,7 +71,7 @@ var extensionTests = []struct { "y-field": &yExtension{}, }, layers: []*inputLayer{ - &inputLayer{ + { order: 1, label: "layer-xy", yaml: ` @@ -83,7 +81,7 @@ var extensionTests = []struct { `, }, }, - errorString: "cannot parse layer .*: unknown section .*", + error: "cannot parse layer .*: unknown section .*", }, // Index 3: Load file layers not unique order { @@ -92,7 +90,7 @@ var extensionTests = []struct { "y-field": &yExtension{}, }, layers: []*inputLayer{ - &inputLayer{ + { order: 1, label: "layer-1", yaml: ` @@ -100,7 +98,7 @@ var extensionTests = []struct { description: desc `, }, - &inputLayer{ + { order: 1, label: "layer-2", yaml: ` @@ -109,7 +107,7 @@ var extensionTests = []struct { `, }, }, - errorString: "invalid layer filename: .* not unique .*", + error: "invalid layer filename: .* not unique .*", }, // Index 4: Load file layers not unique label { @@ -118,7 +116,7 @@ var extensionTests = []struct { "y-field": &yExtension{}, }, layers: []*inputLayer{ - &inputLayer{ + { order: 1, label: "layer-xy", yaml: ` @@ -126,7 +124,7 @@ var extensionTests = []struct { description: desc `, }, - &inputLayer{ + { order: 2, label: "layer-xy", yaml: ` @@ -135,7 +133,7 @@ var extensionTests = []struct { `, }, }, - errorString: "invalid layer filename: .* not unique .*", + error: "invalid layer filename: .* not unique .*", }, // Index 5: Load file layers with empty section { @@ -144,7 +142,7 @@ var extensionTests = []struct { "y-field": &yExtension{}, }, layers: []*inputLayer{ - &inputLayer{ + { order: 1, label: "layer-x", yaml: ` @@ -152,7 +150,7 @@ var extensionTests = []struct { description: desc-x `, }, - &inputLayer{ + { order: 2, label: "layer-y", yaml: ` @@ -161,11 +159,11 @@ var extensionTests = []struct { `, }, }, - combinedPlanResult: &planResult{ + result: &planResult{ x: &xSection{}, y: &ySection{}, }, - combinedPlanResultYaml: string("{}\n"), + resultYaml: "{}\n", }, // Index 6: Load file layers with section validation failure #1 { @@ -174,7 +172,7 @@ var extensionTests = []struct { "y-field": &yExtension{}, }, layers: []*inputLayer{ - &inputLayer{ + { order: 1, label: "layer-x", yaml: ` @@ -188,7 +186,7 @@ var extensionTests = []struct { `, }, }, - errorString: "cannot validate layer section .* cannot accept entry not starting .*", + error: "cannot validate layer section .* cannot accept entry not starting .*", }, // Index 7: Load file layers with section validation failure #2 { @@ -197,7 +195,7 @@ var extensionTests = []struct { "y-field": &yExtension{}, }, layers: []*inputLayer{ - &inputLayer{ + { order: 1, label: "layer-x", yaml: ` @@ -208,7 +206,7 @@ var extensionTests = []struct { `, }, }, - errorString: "cannot validate layer section .* cannot have nil entry .*", + error: "cannot validate layer section .* cannot have nil entry .*", }, // Index 8: Load file layers failed plan validation { @@ -217,7 +215,7 @@ var extensionTests = []struct { "y-field": &yExtension{}, }, layers: []*inputLayer{ - &inputLayer{ + { order: 1, label: "layer-x", yaml: ` @@ -232,7 +230,7 @@ var extensionTests = []struct { - y2 `, }, - &inputLayer{ + { order: 2, label: "layer-y", yaml: ` @@ -246,7 +244,7 @@ var extensionTests = []struct { `, }, }, - errorString: "cannot validate plan section .* cannot find .* as required by .*", + error: "cannot validate plan section .* cannot find .* as required by .*", }, // Index 9: Check empty section omits entry { @@ -255,7 +253,7 @@ var extensionTests = []struct { "y-field": &yExtension{}, }, layers: []*inputLayer{ - &inputLayer{ + { order: 1, label: "layer-x", yaml: ` @@ -264,7 +262,7 @@ var extensionTests = []struct { x-field: `, }, - &inputLayer{ + { order: 2, label: "layer-y", yaml: ` @@ -274,12 +272,11 @@ var extensionTests = []struct { `, }, }, - combinedPlanResult: &planResult{ + result: &planResult{ x: &xSection{}, y: &ySection{}, }, - combinedPlanResultYaml: string(reindent(` - {}`)), + resultYaml: "{}\n", }, // Index 10: Load file layers { @@ -288,7 +285,7 @@ var extensionTests = []struct { "y-field": &yExtension{}, }, layers: []*inputLayer{ - &inputLayer{ + { order: 1, label: "layer-x", yaml: ` @@ -303,7 +300,7 @@ var extensionTests = []struct { - y1 `, }, - &inputLayer{ + { order: 2, label: "layer-y", yaml: ` @@ -317,7 +314,7 @@ var extensionTests = []struct { `, }, }, - combinedPlanResult: &planResult{ + result: &planResult{ x: &xSection{ Entries: map[string]*X{ "x1": &X{ @@ -342,7 +339,7 @@ var extensionTests = []struct { }, }, }, - combinedPlanResultYaml: string(reindent(` + resultYaml: string(reindent(` x-field: x1: override: replace @@ -374,16 +371,16 @@ func (s *S) TestPlanExtensions(c *C) { // Load the plan layer from disk (parse, combine and validate). p, err := plan.ReadDir(layersDir) - if planTest.errorString != "" || err != nil { + if planTest.error != "" || err != nil { // Expected error. - c.Assert(err, ErrorMatches, planTest.errorString) + c.Assert(err, ErrorMatches, planTest.error) } else { if _, ok := planTest.extensions[xField]; ok { // Verify "x-field" data. var x *xSection x = p.Section(xField).(*xSection) c.Assert(err, IsNil) - c.Assert(x.Entries, DeepEquals, planTest.combinedPlanResult.x.Entries) + c.Assert(x.Entries, DeepEquals, planTest.result.x.Entries) } if _, ok := planTest.extensions[yField]; ok { @@ -391,13 +388,13 @@ func (s *S) TestPlanExtensions(c *C) { var y *ySection y = p.Section(yField).(*ySection) c.Assert(err, IsNil) - c.Assert(y.Entries, DeepEquals, planTest.combinedPlanResult.y.Entries) + c.Assert(y.Entries, DeepEquals, planTest.result.y.Entries) } // Verify combined plan YAML. planYAML, err := yaml.Marshal(p) c.Assert(err, IsNil) - c.Assert(string(planYAML), Equals, planTest.combinedPlanResultYaml) + c.Assert(string(planYAML), Equals, planTest.resultYaml) } // Unregister test extensions. diff --git a/internals/plan/plan.go b/internals/plan/plan.go index fae05339c..b6ddc12ac 100644 --- a/internals/plan/plan.go +++ b/internals/plan/plan.go @@ -50,10 +50,10 @@ type LayerSectionExtension interface { } type LayerSection interface { - // Ask the section to validate itself. + // Validate checks whether the section is valid, returning an error if not. Validate() error - // Returns true if the section is empty. + // IsZero reports whether the section is empty. IsZero() bool } @@ -72,12 +72,11 @@ var layerExtensions = map[string]LayerSectionExtension{} // RegisterExtension adds a plan schema extension. All registrations must be // done before the plan library is used. -func RegisterExtension(field string, ext LayerSectionExtension) error { +func RegisterExtension(field string, ext LayerSectionExtension) { if _, ok := layerExtensions[field]; ok { - return fmt.Errorf("internal error: extension %q already registered", field) + panic(fmt.Sprintf("internal error: extension %q already registered", field)) } layerExtensions[field] = ext - return nil } // UnregisterExtension removes a plan schema extension. This is only @@ -106,22 +105,22 @@ func (p *Plan) Section(field string) LayerSection { // This is required since Sections are based on an inlined map, for which // omitempty and inline together is not currently supported. func (p *Plan) MarshalYAML() (interface{}, error) { - marshalData := make(map[string]interface{}) + data := make(map[string]interface{}) if len(p.Services) != 0 { - marshalData["services"] = p.Services + data["services"] = p.Services } if len(p.LogTargets) != 0 { - marshalData["log-targets"] = p.LogTargets + data["log-targets"] = p.LogTargets } if len(p.Checks) != 0 { - marshalData["checks"] = p.Checks + data["checks"] = p.Checks } for field, section := range p.Sections { if !section.IsZero() { - marshalData[field] = section + data[field] = section } } - return marshalData, nil + return data, nil } type Layer struct { @@ -654,7 +653,7 @@ func CombineLayers(layers ...*Layer) (*Layer, error) { combined.Sections[field], err = extension.CombineSections(sections...) if err != nil { return nil, &FormatError{ - Message: fmt.Sprintf(`cannot combine section %q: %v`, field, err), + Message: fmt.Sprintf("cannot combine section %q: %v", field, err), } } } @@ -1259,15 +1258,8 @@ func ParseLayer(order int, label string, data []byte) (*Layer, error) { } } } else { - if extension, ok := layerExtensions[field]; ok { - // Section unmarshal rules are defined by the extension itself. - layer.Sections[field], err = extension.ParseSection(section) - if err != nil { - return nil, &FormatError{ - Message: fmt.Sprintf("cannot parse layer %q section %q: %v", label, field, err), - } - } - } else { + extension, ok := layerExtensions[field] + if !ok { // At the top level we do not ignore keys we do not understand. // This preserves the current Pebble behaviour of decoding with // KnownFields = true. @@ -1275,6 +1267,14 @@ func ParseLayer(order int, label string, data []byte) (*Layer, error) { Message: fmt.Sprintf("cannot parse layer %q: unknown section %q", label, field), } } + + // Section unmarshal rules are defined by the extension itself. + layer.Sections[field], err = extension.ParseSection(section) + if err != nil { + return nil, &FormatError{ + Message: fmt.Sprintf("cannot parse layer %q section %q: %v", label, field, err), + } + } } }