update add apis - process everything before return an error

pkg/devfile/parser/data/v2/commands.go

@@ -5,6 +5,7 @@ import (
 	v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
 	"reflect"
+	"strings"
 )
 
 // GetCommands returns the slice of Command objects parsed from the Devfile
@@ -49,17 +50,22 @@ func (d *DevfileV2) GetCommands(options common.DevfileOptions) ([]v1.Command, er
 }
 
 // AddCommands adds the slice of Command objects to the Devfile's commands
-// if a command is already defined, error out
+// a command is considered as invalid if it is already defined
+// command list passed in will be all processed, and returns a total error of all invalid commands
 func (d *DevfileV2) AddCommands(commands []v1.Command) error {
-
+	var errorsList []string
 	for _, command := range commands {
 		for _, devfileCommand := range d.Commands {
 			if command.Id == devfileCommand.Id {
-				return &common.FieldAlreadyExistError{Name: command.Id, Field: "command"}
+				errorsList = append(errorsList, (&common.FieldAlreadyExistError{Name: command.Id, Field: "command"}).Error())
+				continue
 			}
 		}
 		d.Commands = append(d.Commands, command)
 	}
+	if len(errorsList) > 0 {
+		return fmt.Errorf("errors while adding commands:\n%s", strings.Join(errorsList, "\n"))
+	}
 	return nil
 }
 

pkg/devfile/parser/data/v2/commands_test.go

@@ -1,6 +1,8 @@
 package v2
 
 import (
+	"fmt"
+	"github.com/kylelemons/godebug/pretty"
 	"reflect"
 	"testing"
 
@@ -226,15 +228,13 @@ func TestDevfile200_GetCommands(t *testing.T) {
 }
 
 func TestDevfile200_AddCommands(t *testing.T) {
+	multipleDupError := fmt.Sprintf("%s\n%s", "command command1 already exists in devfile", "command command2 already exists in devfile")
 
-	type args struct {
-		name string
-	}
 	tests := []struct {
 		name            string
 		currentCommands []v1.Command
 		newCommands     []v1.Command
-		wantErr         bool
+		wantErr         *string
 	}{
 		{
 			name: "Command does not exist",
@@ -260,17 +260,23 @@ func TestDevfile200_AddCommands(t *testing.T) {
 					},
 				},
 			},
-			wantErr: false,
+			wantErr: nil,
 		},
 		{
-			name: "Command does exist",
+			name: "Multiple duplicate commands",
 			currentCommands: []v1.Command{
 				{
 					Id: "command1",
 					CommandUnion: v1.CommandUnion{
 						Exec: &v1.ExecCommand{},
 					},
 				},
+				{
+					Id: "command2",
+					CommandUnion: v1.CommandUnion{
+						Exec: &v1.ExecCommand{},
+					},
+				},
 			},
 			newCommands: []v1.Command{
 				{
@@ -279,8 +285,20 @@ func TestDevfile200_AddCommands(t *testing.T) {
 						Exec: &v1.ExecCommand{},
 					},
 				},
+				{
+					Id: "command2",
+					CommandUnion: v1.CommandUnion{
+						Exec: &v1.ExecCommand{},
+					},
+				},
+				{
+					Id: "command3",
+					CommandUnion: v1.CommandUnion{
+						Exec: &v1.ExecCommand{},
+					},
+				},
 			},
-			wantErr: true,
+			wantErr: &multipleDupError,
 		},
 	}
 	for _, tt := range tests {
@@ -297,9 +315,17 @@ func TestDevfile200_AddCommands(t *testing.T) {
 
 			err := d.AddCommands(tt.newCommands)
 			// Unexpected error
-			if (err != nil) != tt.wantErr {
+			if (err != nil) != (tt.wantErr != nil) {
 				t.Errorf("TestDevfile200_AddCommands() error = %v, wantErr %v", err, tt.wantErr)
+			} else if tt.wantErr != nil {
+				assert.Regexp(t, *tt.wantErr, err.Error(), "Error message should match")
+			} else {
+				wantCommands := append(tt.currentCommands, tt.newCommands...)
+				if !reflect.DeepEqual(d.Commands, wantCommands) {
+					t.Errorf("TestDevfile200_AddCommands() wanted: %v, got: %v, difference at %v", wantCommands, d.Commands, pretty.Compare(wantCommands, d.Commands))
+				}
 			}
+
 		})
 	}
 }

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

@@ -3,6 +3,7 @@ package v2
 import (
 	"fmt"
 	"reflect"
+	"strings"
 
 	v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
@@ -73,17 +74,22 @@ func (d *DevfileV2) GetDevfileVolumeComponents(options common.DevfileOptions) ([
 }
 
 // AddComponents adds the slice of Component objects to the devfile's components
-// if a component is already defined, error out
+// a component is considered as invalid if it is already defined
+// component list passed in will be all processed, and returns a total error of all invalid components
 func (d *DevfileV2) AddComponents(components []v1.Component) error {
-
+	var errorsList []string
 	for _, component := range components {
 		for _, devfileComponent := range d.Components {
 			if component.Name == devfileComponent.Name {
-				return &common.FieldAlreadyExistError{Name: component.Name, Field: "component"}
+				errorsList = append(errorsList, (&common.FieldAlreadyExistError{Name: component.Name, Field: "component"}).Error())
+				continue
 			}
 		}
 		d.Components = append(d.Components, component)
 	}
+	if len(errorsList) > 0 {
+		return fmt.Errorf("errors while adding components:\n%s", strings.Join(errorsList, "\n"))
+	}
 	return nil
 }
 

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

@@ -1,6 +1,8 @@
 package v2
 
 import (
+	"fmt"
+	"github.com/kylelemons/godebug/pretty"
 	"reflect"
 	"testing"
 
@@ -12,12 +14,13 @@ import (
 )
 
 func TestDevfile200_AddComponent(t *testing.T) {
+	multipleDupError := fmt.Sprintf("%s\n%s", "component component1 already exists in devfile", "component component2 already exists in devfile")
 
 	tests := []struct {
 		name              string
 		currentComponents []v1.Component
 		newComponents     []v1.Component
-		wantErr           bool
+		wantErr           *string
 	}{
 		{
 			name: "successfully add the component",
@@ -43,7 +46,7 @@ func TestDevfile200_AddComponent(t *testing.T) {
 					},
 				},
 			},
-			wantErr: false,
+			wantErr: nil,
 		},
 		{
 			name: "error out on duplicate component",
@@ -68,8 +71,20 @@ func TestDevfile200_AddComponent(t *testing.T) {
 						Container: &v1.ContainerComponent{},
 					},
 				},
+				{
+					Name: "component2",
+					ComponentUnion: v1.ComponentUnion{
+						Volume: &v1.VolumeComponent{},
+					},
+				},
+				{
+					Name: "component3",
+					ComponentUnion: v1.ComponentUnion{
+						Volume: &v1.VolumeComponent{},
+					},
+				},
 			},
-			wantErr: true,
+			wantErr: &multipleDupError,
 		},
 	}
 	for _, tt := range tests {
@@ -86,8 +101,15 @@ func TestDevfile200_AddComponent(t *testing.T) {
 
 			err := d.AddComponents(tt.newComponents)
 			// Unexpected error
-			if (err != nil) != tt.wantErr {
+			if (err != nil) != (tt.wantErr != nil) {
 				t.Errorf("TestDevfile200_AddComponents() error = %v, wantErr %v", err, tt.wantErr)
+			} else if tt.wantErr != nil {
+				assert.Regexp(t, *tt.wantErr, err.Error(), "Error message should match")
+			} else {
+				wantComponents := append(tt.currentComponents, tt.newComponents...)
+				if !reflect.DeepEqual(d.Components, wantComponents) {
+					t.Errorf("TestDevfile200_AddComponents() wanted: %v, got: %v, difference at %v", wantComponents, d.Components, pretty.Compare(wantComponents, d.Components))
+				}
 			}
 		})
 	}

pkg/devfile/parser/data/v2/events.go

@@ -1,8 +1,10 @@
 package v2
 
 import (
+	"fmt"
 	v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
+	"strings"
 )
 
 // GetEvents returns the Events Object parsed from devfile
@@ -14,41 +16,48 @@ func (d *DevfileV2) GetEvents() v1.Events {
 }
 
 // AddEvents adds the Events Object to the devfile's events
-// if the event is already defined in the devfile, error out
+// an event field is considered as invalid if it is already defined
+// all event fields will be checked and processed, and returns a total error of all event fields
 func (d *DevfileV2) AddEvents(events v1.Events) error {
 
 	if d.Events == nil {
 		d.Events = &v1.Events{}
 	}
-
+	var errorsList []string
 	if len(events.PreStop) > 0 {
 		if len(d.Events.PreStop) > 0 {
-			return &common.FieldAlreadyExistError{Field: "pre stop"}
+			errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "pre stop"}).Error())
+		} else {
+			d.Events.PreStop = events.PreStop
 		}
-		d.Events.PreStop = events.PreStop
 	}
 
 	if len(events.PreStart) > 0 {
 		if len(d.Events.PreStart) > 0 {
-			return &common.FieldAlreadyExistError{Field: "pre start"}
+			errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "pre start"}).Error())
+		} else {
+			d.Events.PreStart = events.PreStart
 		}
-		d.Events.PreStart = events.PreStart
 	}
 
 	if len(events.PostStop) > 0 {
 		if len(d.Events.PostStop) > 0 {
-			return &common.FieldAlreadyExistError{Field: "post stop"}
+			errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "post stop"}).Error())
+		} else {
+			d.Events.PostStop = events.PostStop
 		}
-		d.Events.PostStop = events.PostStop
 	}
 
 	if len(events.PostStart) > 0 {
 		if len(d.Events.PostStart) > 0 {
-			return &common.FieldAlreadyExistError{Field: "post start"}
+			errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "post start"}).Error())
+		} else {
+			d.Events.PostStart = events.PostStart
 		}
-		d.Events.PostStart = events.PostStart
 	}
-
+	if len(errorsList) > 0 {
+		return fmt.Errorf("errors while adding events:\n%s", strings.Join(errorsList, "\n"))
+	}
 	return nil
 }
 

pkg/devfile/parser/data/v2/events_test.go

@@ -1,19 +1,24 @@
 package v2
 
 import (
+	"fmt"
+	"github.com/kylelemons/godebug/pretty"
+	"github.com/stretchr/testify/assert"
 	"reflect"
 	"testing"
 
 	v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 )
 
 func TestDevfile200_AddEvents(t *testing.T) {
+	multipleDupError := fmt.Sprintf("%s\n%s", "event field pre start already exists in devfile", "event field post stop already exists in devfile")
 
 	tests := []struct {
 		name          string
 		currentEvents *v1.Events
 		newEvents     v1.Events
-		wantErr       bool
+		wantEvents    v1.Events
+		wantErr       *string
 	}{
 		{
 			name: "successfully add the events",
@@ -27,7 +32,13 @@ func TestDevfile200_AddEvents(t *testing.T) {
 					PostStart: []string{"postStart1"},
 				},
 			},
-			wantErr: false,
+			wantEvents: v1.Events{
+				DevWorkspaceEvents: v1.DevWorkspaceEvents{
+					PreStart:  []string{"preStart1"},
+					PostStart: []string{"postStart1"},
+				},
+			},
+			wantErr: nil,
 		},
 		{
 			name:          "successfully add the events to empty devfile event",
@@ -37,21 +48,29 @@ func TestDevfile200_AddEvents(t *testing.T) {
 					PostStart: []string{"postStart1"},
 				},
 			},
-			wantErr: false,
+			wantEvents: v1.Events{
+				DevWorkspaceEvents: v1.DevWorkspaceEvents{
+					PostStart: []string{"postStart1"},
+				},
+			},
+			wantErr: nil,
 		},
 		{
 			name: "event already present",
 			currentEvents: &v1.Events{
 				DevWorkspaceEvents: v1.DevWorkspaceEvents{
 					PreStart: []string{"preStart1"},
+					PostStop: []string{"postStop1"},
 				},
 			},
 			newEvents: v1.Events{
 				DevWorkspaceEvents: v1.DevWorkspaceEvents{
 					PreStart: []string{"preStart2"},
+					PostStop: []string{"postStop2"},
+					PreStop:  []string{"preStop"},
 				},
 			},
-			wantErr: true,
+			wantErr: &multipleDupError,
 		},
 	}
 	for _, tt := range tests {
@@ -68,8 +87,14 @@ func TestDevfile200_AddEvents(t *testing.T) {
 
 			err := d.AddEvents(tt.newEvents)
 
-			if (err != nil) != tt.wantErr {
-				t.Errorf("DeleteCommand() error = %v, wantErr %v", err, tt.wantErr)
+			if (err != nil) != (tt.wantErr != nil) {
+				t.Errorf("TestDevfile200_AddEvents() error = %v, wantErr %v", err, tt.wantErr)
+			} else if tt.wantErr != nil {
+				assert.Regexp(t, *tt.wantErr, err.Error(), "Error message should match")
+			} else {
+				if !reflect.DeepEqual(*d.Events, tt.wantEvents) {
+					t.Errorf("TestDevfile200_AddEvents() wanted: %v, got: %v, difference at %v", tt.wantEvents, *d.Events, pretty.Compare(tt.wantEvents, *d.Events))
+				}
 			}
 
 		})