Improve validation error messaging

Author: lindenmckenzie

domain/job_validator.go

@@ -59,8 +59,8 @@ func (v *StaticDatasetValidator) ValidateSourceIDWithExternal(ctx context.Contex
 	}
 
 	if data.Type != zebedee.PageTypeDatasetLandingPage {
-		log.Error(ctx, data.Type, appErrors.ErrSourceIDInvalid)
-		return "", appErrors.ErrSourceIDInvalid
+		log.Error(ctx, data.Type, appErrors.ErrSourceDataTypeInvalid)
+		return "", appErrors.ErrSourceDataTypeInvalid
 	}
 
 	// Extract and validate title
@@ -89,7 +89,7 @@ func checkZebedeeURIExists(ctx context.Context, client clients.ZebedeeClient, ur
 	if err != nil {
 		if errors.As(err, &e) {
 			if e.ActualCode == http.StatusNotFound {
-				return zebedee.PageData{}, appErrors.ErrSourceIDInvalid
+				return zebedee.PageData{}, appErrors.ErrSourceDoesNotExist
 			}
 		}
 		log.Error(ctx, "failed to validate source ID with zebedee", err)
@@ -108,7 +108,7 @@ func checkDatasetIDDoesNotExist(ctx context.Context, client datasetSDK.Clienter,
 		return appErrors.ErrTargetIDValidation
 	}
 
-	return appErrors.ErrTargetIDInvalid
+	return appErrors.ErrTargetAlreadyExists
 }
 
 // ValidateZebedeeURI validates if the given path is a valid URI

domain/job_validator_test.go

@@ -33,12 +33,16 @@ const (
 	datasetValidID    = "found"
 )
 
+var (
+	errTest = errors.New("unexpected error")
+)
+
 func TestStaticDatasetValidatorWithExternal(t *testing.T) {
 	zebedeeMock := &clientMocks.ZebedeeClientMock{
 		GetPageDataFunc: func(ctx context.Context, userAuthToken, collectionID, lang, path string) (zebedee.PageData, error) {
 			switch path {
 			case zebedeeErrorPath:
-				return zebedee.PageData{}, errors.New("unexpected error")
+				return zebedee.PageData{}, errTest
 			case zebedeeValidPath:
 				return zebedee.PageData{
 					Type: zebedee.PageTypeDatasetLandingPage,
@@ -71,7 +75,7 @@ func TestStaticDatasetValidatorWithExternal(t *testing.T) {
 		GetDatasetFunc: func(ctx context.Context, headers datasetSDK.Headers, datasetID string) (datasetModels.Dataset, error) {
 			switch datasetID {
 			case datasetErrorID:
-				return datasetModels.Dataset{}, errors.New("unexpected error")
+				return datasetModels.Dataset{}, errTest
 			case datasetValidID:
 				return datasetModels.Dataset{}, nil
 			case datasetNotFoundID:
@@ -129,6 +133,7 @@ func TestStaticDatasetValidatorWithExternal(t *testing.T) {
 
 			Convey("Then an error should be returned", func() {
 				So(err, ShouldNotBeNil)
+				So(err, ShouldEqual, appErrors.ErrSourceIDValidation)
 
 				Convey("And the title should be empty", func() {
 					So(title, ShouldEqual, "")
@@ -137,14 +142,15 @@ func TestStaticDatasetValidatorWithExternal(t *testing.T) {
 		})
 	})
 
-	Convey("Given a zebedee source ID that returns not found error", t, func() {
+	Convey("Given a zebedee source ID that returns a not found error", t, func() {
 		validator := domain.StaticDatasetValidator{}
 
 		Convey("When the source is validated", func() {
 			title, err := validator.ValidateSourceIDWithExternal(ctx, zebedeeNotFoundPath, &mockClientlist, testUserAuthToken)
 
 			Convey("Then an error should be returned", func() {
 				So(err, ShouldNotBeNil)
+				So(err, ShouldEqual, appErrors.ErrSourceDoesNotExist)
 
 				Convey("And the title should be empty", func() {
 					So(title, ShouldEqual, "")
@@ -161,6 +167,7 @@ func TestStaticDatasetValidatorWithExternal(t *testing.T) {
 
 			Convey("Then an error should be returned", func() {
 				So(err, ShouldNotBeNil)
+				So(err, ShouldEqual, appErrors.ErrSourceDataTypeInvalid)
 
 				Convey("And the title should be empty", func() {
 					So(title, ShouldEqual, "")
@@ -189,6 +196,7 @@ func TestStaticDatasetValidatorWithExternal(t *testing.T) {
 
 			Convey("Then an error should be returned", func() {
 				So(err, ShouldNotBeNil)
+				So(err, ShouldEqual, appErrors.ErrTargetAlreadyExists)
 			})
 		})
 	})
@@ -201,6 +209,7 @@ func TestStaticDatasetValidatorWithExternal(t *testing.T) {
 
 			Convey("Then an error should be returned", func() {
 				So(err, ShouldNotBeNil)
+				So(err, ShouldEqual, appErrors.ErrTargetIDValidation)
 			})
 		})
 	})

errors/errors.go

@@ -42,8 +42,8 @@ var (
 	ErrTargetIDNotProvided          = errors.New("target ID not provided")
 	ErrJobTypeNotProvided           = errors.New("job type not provided")
 	ErrSourceTitleNotFound          = errors.New("source title not found or empty")
-	ErrSourceIDInvalid              = errors.New("source ID is invalid")
-	ErrTargetIDInvalid              = errors.New("target ID is invalid")
+	ErrSourceDoesNotExist           = errors.New("source ID does not exist")
+	ErrTargetAlreadyExists          = errors.New("target ID already exists")
 	ErrJobTypeInvalid               = errors.New("job type is invalid")
 	ErrInternalServerError          = errors.New("an unexpected error occurred")
 	ErrSourceIDValidation           = errors.New("source ID failed to validate")
@@ -88,8 +88,9 @@ var (
 		ErrTargetIDValidation:           http.StatusInternalServerError,
 		ErrJobNumberCounterNotFound:     http.StatusInternalServerError,
 		ErrJobAlreadyRunning:            http.StatusConflict,
-		ErrSourceIDInvalid:              http.StatusBadRequest,
-		ErrTargetIDInvalid:              http.StatusBadRequest,
+		ErrSourceDoesNotExist:           http.StatusBadRequest,
+		ErrTargetAlreadyExists:          http.StatusBadRequest,
+		ErrSourceDataTypeInvalid:        http.StatusBadRequest,
 		ErrJobTypeInvalid:               http.StatusBadRequest,
 		ErrSourceIDZebedeeURIInvalid:    http.StatusBadRequest,
 		ErrTargetIDDatasetIDInvalid:     http.StatusBadRequest,
@@ -99,8 +100,6 @@ var (
 		ErrLimitInvalid:                 http.StatusBadRequest,
 		ErrLimitExceeded:                http.StatusBadRequest,
 		ErrJobStateTransitionNotAllowed: http.StatusConflict,
-		ErrSourceIDInvalid:              http.StatusBadRequest,
-		ErrTargetIDInvalid:              http.StatusBadRequest,
 		ErrJobTypeInvalid:               http.StatusBadRequest,
 		ErrJobStateNotAllowed:           http.StatusBadRequest,
 		ErrSortFieldInvalid:             http.StatusBadRequest,

features/create_job.feature

@@ -144,7 +144,7 @@ Feature: Create a Job
           "errors": [
             {
               "code": 400,
-              "description": "target ID is invalid"
+              "description": "target ID already exists"
             }
           ]
         }
@@ -175,7 +175,7 @@ Feature: Create a Job
           "errors": [
             {
               "code": 400,
-              "description": "source ID is invalid"
+              "description": "source data has incorrect type"
             }
           ]
         }
@@ -207,7 +207,7 @@ Feature: Create a Job
           "errors": [
             {
               "code": 400,
-              "description": "target ID is invalid"
+              "description": "target ID already exists"
             }
           ]
         }