Skip to content

Commit 70e85e6

Browse files
yangcao77yangcao77
committed
address review comments
Signed-off-by: Stephanie <[email protected]>
1 parent be272c3 commit 70e85e6

File tree

7 files changed

+123
-75
lines changed

7 files changed

+123
-75
lines changed

‎pkg/devfile/parser/data/v2/common/options.go

+4-5
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ package common
33
import (
44
"reflect"
55

6-
"github.com/devfile/api/pkg/attributes"
7-
attriutespkg "github.com/devfile/api/pkg/attributes"
6+
apiAttributes "github.com/devfile/api/pkg/attributes"
87
)
98

109
// DevfileOptions provides options for Devfile operations
@@ -14,14 +13,14 @@ type DevfileOptions struct {
1413
}
1514

1615
// FilterDevfileObject filters devfile attributes with the given options
17-
func FilterDevfileObject(attributes attributes.Attributes, options DevfileOptions) (bool, error) {
16+
func FilterDevfileObject(attributes apiAttributes.Attributes, options DevfileOptions) (bool, error) {
1817
var err error
1918
filterIn := true
2019
for key, value := range options.Filter {
2120
currentFilterIn := false
2221
attrValue := attributes.Get(key, &err)
23-
var keynotfound = &attriutespkg.KeyNotFoundError{Key: key}
24-
if err != nil && err.Error() != keynotfound.Error() {
22+
var keyNotFoundErr = &apiAttributes.KeyNotFoundError{Key: key}
23+
if err != nil && err.Error() != keyNotFoundErr.Error() {
2524
return false, err
2625
} else if reflect.DeepEqual(attrValue, value) {
2726
currentFilterIn = true

‎pkg/devfile/parser/data/v2/components.go

+6-38
Original file line numberDiff line numberDiff line change
@@ -60,48 +60,16 @@ func (d *DevfileV2) GetDevfileVolumeComponents(options common.DevfileOptions) ([
6060
// if a component is already defined, error out
6161
func (d *DevfileV2) AddComponents(components []v1.Component) error {
6262

63-
// different map for volume and container component as a volume and a container with same name
64-
// can exist in devfile
65-
containerMap := make(map[string]bool)
66-
volumeMap := make(map[string]bool)
67-
pluginMap := make(map[string]bool)
63+
componentMap := make(map[string]bool)
6864

6965
for _, component := range d.Components {
70-
if component.Volume != nil {
71-
volumeMap[component.Name] = true
72-
}
73-
if component.Container != nil {
74-
containerMap[component.Name] = true
75-
}
76-
if component.Plugin != nil {
77-
pluginMap[component.Name] = true
78-
}
66+
componentMap[component.Name] = true
7967
}
80-
8168
for _, component := range components {
82-
83-
if component.Volume != nil {
84-
if _, ok := volumeMap[component.Name]; !ok {
85-
d.Components = append(d.Components, component)
86-
} else {
87-
return &common.FieldAlreadyExistError{Name: component.Name, Field: "component"}
88-
}
89-
}
90-
91-
if component.Container != nil {
92-
if _, ok := containerMap[component.Name]; !ok {
93-
d.Components = append(d.Components, component)
94-
} else {
95-
return &common.FieldAlreadyExistError{Name: component.Name, Field: "component"}
96-
}
97-
}
98-
99-
if component.Plugin != nil {
100-
if _, ok := pluginMap[component.Name]; !ok {
101-
d.Components = append(d.Components, component)
102-
} else {
103-
return &common.FieldAlreadyExistError{Name: component.Name, Field: "component"}
104-
}
69+
if _, ok := componentMap[component.Name]; !ok {
70+
d.Components = append(d.Components, component)
71+
} else {
72+
return &common.FieldAlreadyExistError{Name: component.Name, Field: "component"}
10573
}
10674
}
10775
return nil

‎pkg/devfile/parser/data/v2/components_test.go

-6
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,6 @@ func TestDevfile200_AddComponent(t *testing.T) {
3535
},
3636
},
3737
newComponents: []v1.Component{
38-
{
39-
Name: "component2",
40-
ComponentUnion: v1.ComponentUnion{
41-
Container: &v1.ContainerComponent{},
42-
},
43-
},
4438
{
4539
Name: "component3",
4640
ComponentUnion: v1.ComponentUnion{

‎pkg/devfile/parser/data/v2/types.go

-11
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,3 @@ import (
88
type DevfileV2 struct {
99
v1.Devfile
1010
}
11-
12-
// GetDevfileWorkspace returns the workspace content for the devfile
13-
func (d *DevfileV2) GetDevfileWorkspace() *v1.DevWorkspaceTemplateSpecContent {
14-
15-
return &d.DevWorkspaceTemplateSpecContent
16-
}
17-
18-
// SetDevfileWorkspace sets the workspace content
19-
func (d *DevfileV2) SetDevfileWorkspace(content v1.DevWorkspaceTemplateSpecContent) {
20-
d.DevWorkspaceTemplateSpecContent = content
21-
}

‎pkg/devfile/parser/data/v2/workspace.go

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package v2
2+
3+
import (
4+
v1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
5+
)
6+
7+
// GetDevfileWorkspace returns the workspace content for the devfile
8+
func (d *DevfileV2) GetDevfileWorkspace() *v1.DevWorkspaceTemplateSpecContent {
9+
10+
return &d.DevWorkspaceTemplateSpecContent
11+
}
12+
13+
// SetDevfileWorkspace sets the workspace content
14+
func (d *DevfileV2) SetDevfileWorkspace(content v1.DevWorkspaceTemplateSpecContent) {
15+
d.DevWorkspaceTemplateSpecContent = content
16+
}

‎pkg/devfile/parser/data/v2/workspace_test.go

+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package v2
2+
3+
import (
4+
"reflect"
5+
"testing"
6+
7+
v1 "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
8+
)
9+
10+
func TestDevfile200_SetDevfileWorkspace(t *testing.T) {
11+
12+
type args struct {
13+
name string
14+
}
15+
tests := []struct {
16+
name string
17+
workspace v1.DevWorkspaceTemplateSpecContent
18+
devfilev2 *DevfileV2
19+
expectedDevfilev2 *DevfileV2
20+
}{
21+
{
22+
name: "set workspace",
23+
devfilev2: &DevfileV2{
24+
v1.Devfile{},
25+
},
26+
workspace: v1.DevWorkspaceTemplateSpecContent{
27+
Components: []v1.Component{
28+
{
29+
Name: "component1",
30+
ComponentUnion: v1.ComponentUnion{
31+
Container: &v1.ContainerComponent{},
32+
},
33+
},
34+
{
35+
Name: "component2",
36+
ComponentUnion: v1.ComponentUnion{
37+
Volume: &v1.VolumeComponent{},
38+
},
39+
},
40+
},
41+
},
42+
expectedDevfilev2: &DevfileV2{
43+
v1.Devfile{
44+
DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{
45+
DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{
46+
Components: []v1.Component{
47+
{
48+
Name: "component1",
49+
ComponentUnion: v1.ComponentUnion{
50+
Container: &v1.ContainerComponent{},
51+
},
52+
},
53+
{
54+
Name: "component2",
55+
ComponentUnion: v1.ComponentUnion{
56+
Volume: &v1.VolumeComponent{},
57+
},
58+
},
59+
},
60+
},
61+
},
62+
},
63+
},
64+
},
65+
}
66+
for _, tt := range tests {
67+
t.Run(tt.name, func(t *testing.T) {
68+
tt.devfilev2.SetDevfileWorkspace(tt.workspace)
69+
if !reflect.DeepEqual(tt.devfilev2, tt.expectedDevfilev2) {
70+
t.Errorf("TestDevfile200_SetDevfileWorkspace() expected %v, got %v", tt.expectedDevfilev2, tt.devfilev2)
71+
}
72+
})
73+
}
74+
}

‎pkg/devfile/parser/parse.go

+23-15
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package parser
22

33
import (
44
"encoding/json"
5+
"fmt"
56

67
devfileCtx "github.com/devfile/library/pkg/devfile/parser/context"
78
"github.com/devfile/library/pkg/devfile/parser/data"
@@ -63,7 +64,7 @@ func Parse(path string) (d DevfileObj, err error) {
6364
return parseDevfile(d, true)
6465
}
6566

66-
// ParseRawDevfile populates the raw devfile data witout overring and merging
67+
// ParseRawDevfile populates the raw devfile data without overriding and merging
6768
func ParseRawDevfile(path string) (d DevfileObj, err error) {
6869
// NewDevfileCtx
6970
d.Ctx = devfileCtx.NewDevfileCtx(path)
@@ -108,15 +109,20 @@ func ParseFromData(data []byte) (d DevfileObj, err error) {
108109
func parseParentAndPlugin(d DevfileObj) (err error) {
109110
flattenedParent := &v1.DevWorkspaceTemplateSpecContent{}
110111
if d.Data.GetParent() != nil {
111-
if !reflect.DeepEqual(d.Data.GetParent(), &v1.Parent{}) && d.Data.GetParent().Uri != "" {
112-
parent := d.Data.GetParent()
112+
if !reflect.DeepEqual(d.Data.GetParent(), &v1.Parent{}) {
113113

114-
parentData, err := ParseFromURL(parent.Uri)
115-
if err != nil {
116-
return err
114+
parent := d.Data.GetParent()
115+
var parentDevfileObj DevfileObj
116+
if d.Data.GetParent().Uri != "" {
117+
parentDevfileObj, err = ParseFromURL(parent.Uri)
118+
if err != nil {
119+
return err
120+
}
121+
} else {
122+
return fmt.Errorf("parent URI undefined, currently only URI is suppported")
117123
}
118124

119-
parentWorkspaceContent := parentData.Data.GetDevfileWorkspace()
125+
parentWorkspaceContent := parentDevfileObj.Data.GetDevfileWorkspace()
120126
if !reflect.DeepEqual(parent.ParentOverrides, v1.ParentOverrides{}) {
121127
flattenedParent, err = apiOverride.OverrideDevWorkspaceTemplateSpec(parentWorkspaceContent, parent.ParentOverrides)
122128
if err != nil {
@@ -129,33 +135,35 @@ func parseParentAndPlugin(d DevfileObj) (err error) {
129135
klog.V(4).Infof("adding data of devfile with URI: %v", parent.Uri)
130136
}
131137
}
132-
plugins := []*v1.DevWorkspaceTemplateSpecContent{}
138+
flattenedPlugins := []*v1.DevWorkspaceTemplateSpecContent{}
133139
components, err := d.Data.GetComponents(common.DevfileOptions{})
134140
if err != nil {
135141
return err
136142
}
137143
for _, component := range components {
138144
if component.Plugin != nil && !reflect.DeepEqual(component.Plugin, &v1.PluginComponent{}) {
139145
plugin := component.Plugin
140-
var pluginData DevfileObj
146+
var pluginDevfileObj DevfileObj
141147
if plugin.Uri != "" {
142-
pluginData, err = ParseFromURL(plugin.Uri)
148+
pluginDevfileObj, err = ParseFromURL(plugin.Uri)
143149
if err != nil {
144150
return err
145151
}
152+
} else {
153+
return fmt.Errorf("parent URI undefined, currently only URI is suppported")
146154
}
147-
pluginWorkspaceContent := pluginData.Data.GetDevfileWorkspace()
148-
result := pluginWorkspaceContent
155+
pluginWorkspaceContent := pluginDevfileObj.Data.GetDevfileWorkspace()
156+
flattenedPlugin := pluginWorkspaceContent
149157
if !reflect.DeepEqual(plugin.PluginOverrides, v1.PluginOverrides{}) {
150-
result, err = apiOverride.OverrideDevWorkspaceTemplateSpec(pluginWorkspaceContent, plugin.PluginOverrides)
158+
flattenedPlugin, err = apiOverride.OverrideDevWorkspaceTemplateSpec(pluginWorkspaceContent, plugin.PluginOverrides)
151159
if err != nil {
152160
return err
153161
}
154162
}
155-
plugins = append(plugins, result)
163+
flattenedPlugins = append(flattenedPlugins, flattenedPlugin)
156164
}
157165
}
158-
mergedContent, err := apiOverride.MergeDevWorkspaceTemplateSpec(d.Data.GetDevfileWorkspace(), flattenedParent, plugins...)
166+
mergedContent, err := apiOverride.MergeDevWorkspaceTemplateSpec(d.Data.GetDevfileWorkspace(), flattenedParent, flattenedPlugins...)
159167
if err != nil {
160168
return err
161169
}

0 commit comments

Comments
 (0)