Add validation tests for topology

See merge request !573

controller/northbound/server/topology.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"context"
+	"errors"
 	"time"
 
 	topopb "code.fbi.h-da.de/danet/gosdn/api/go/gosdn/topology"
@@ -15,6 +16,7 @@ import (
 	"github.com/google/uuid"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/reflect/protoreflect"
 )
 
 // TopologyServer holds a topologyService and represents a TopologyServiceServer.
@@ -41,10 +43,29 @@ func NewTopologyServer(
 	}
 }
 
+func (t TopologyServer) checkForValidationErrors(request protoreflect.ProtoMessage) error {
+	err := t.protoValidator.Validate(request)
+	if err != nil {
+		var valErr *protovalidate.ValidationError
+
+		if ok := errors.As(err, &valErr); ok {
+			protoErr := valErr.ToProto()
+			grpcError, _ := status.New(codes.Aborted, "Validation failed").WithDetails(protoErr)
+
+			return grpcError.Err()
+		}
+
+		return status.Errorf(codes.Aborted, "%v", err)
+	}
+
+	return nil
+}
+
 // AddLink adds a new link to the topology.
 func (t *TopologyServer) AddLink(ctx context.Context, request *topopb.AddLinkRequest) (*topopb.AddLinkResponse, error) {
-	if err := t.protoValidator.Validate(request); err != nil {
-		return nil, status.Errorf(codes.Aborted, "%v", err)
+	err := t.checkForValidationErrors(request)
+	if err != nil {
+		return nil, err
 	}
 
 	sourceNode, sourcePort, err := t.ensureNodeAndPortExists(request.Link.SourceNode, request.Link.SourcePort)
@@ -77,8 +98,9 @@ func (t *TopologyServer) AddLink(ctx context.Context, request *topopb.AddLinkReq
 
 // GetTopology returns the current topology in the form of all links.
 func (t *TopologyServer) GetTopology(ctx context.Context, request *topopb.GetTopologyRequest) (*topopb.GetTopologyResponse, error) {
-	if err := t.protoValidator.Validate(request); err != nil {
-		return nil, status.Errorf(codes.Aborted, "%v", err)
+	err := t.checkForValidationErrors(request)
+	if err != nil {
+		return nil, err
 	}
 
 	topo, err := t.topologyService.GetAll()
@@ -127,8 +149,9 @@ func (t *TopologyServer) GetTopology(ctx context.Context, request *topopb.GetTop
 
 // DeleteLink deletes a link.
 func (t *TopologyServer) DeleteLink(ctx context.Context, request *topopb.DeleteLinkRequest) (*topopb.DeleteLinkResponse, error) {
-	if err := t.protoValidator.Validate(request); err != nil {
-		return nil, status.Errorf(codes.Aborted, "%v", err)
+	err := t.checkForValidationErrors(request)
+	if err != nil {
+		return nil, err
 	}
 
 	linkID, err := uuid.Parse(request.Id)

controller/northbound/server/topology_test.go

@@ -5,6 +5,7 @@ import (
 	"reflect"
 	"testing"
 
+	"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
 	apb "code.fbi.h-da.de/danet/gosdn/api/go/gosdn/topology"
 	eventservice "code.fbi.h-da.de/danet/gosdn/controller/eventService"
 	"code.fbi.h-da.de/danet/gosdn/controller/topology"
@@ -244,11 +245,12 @@ func TestTopology_AddLink(t *testing.T) {
 		request *apb.AddLinkRequest
 	}
 	tests := []struct {
-		name    string
-		fields  fields
-		args    args
-		want    *apb.AddLinkResponse
-		wantErr bool
+		name             string
+		fields           fields
+		args             args
+		want             *apb.AddLinkResponse
+		wantErr          bool
+		validationErrors []*validate.Violation
 	}{
 		{
 			name: "should add a new link",
@@ -281,6 +283,33 @@ func TestTopology_AddLink(t *testing.T) {
 			},
 			want:    &apb.AddLinkResponse{},
 			wantErr: true,
+			validationErrors: []*validate.Violation{
+				{
+					FieldPath:    "link.name",
+					ConstraintId: "string.min_len",
+					Message:      "value length must be at least 1 characters",
+				},
+				{
+					FieldPath:    "link.sourceNode",
+					ConstraintId: "required",
+					Message:      "value is required",
+				},
+				{
+					FieldPath:    "link.targetNode",
+					ConstraintId: "required",
+					Message:      "value is required",
+				},
+				{
+					FieldPath:    "link.sourcePort",
+					ConstraintId: "required",
+					Message:      "value is required",
+				},
+				{
+					FieldPath:    "link.targetPort",
+					ConstraintId: "required",
+					Message:      "value is required",
+				},
+			},
 		},
 	}
 	for _, tt := range tests {
@@ -289,7 +318,8 @@ func TestTopology_AddLink(t *testing.T) {
 			_, err := tr.AddLink(tt.args.ctx, tt.args.request)
 			if err != nil {
 				if tt.wantErr {
-					// TODO: check error
+					assertValidationErrors(t, err, tt.validationErrors)
+
 					return
 				}
 
@@ -318,7 +348,7 @@ func TestTopology_GetTopology(t *testing.T) {
 		wantErr bool
 	}{
 		{
-			name: "should add a new link",
+			name: "should get an existing link",
 			fields: fields{
 				ports: []ports.Port{getTestSourcePort(), getTestTargetPort()},
 				nodes: []nodes.Node{getTestSourceNode(), getTestTargetNode()},
@@ -391,14 +421,15 @@ func TestTopology_DeleteLink(t *testing.T) {
 		request *apb.DeleteLinkRequest
 	}
 	tests := []struct {
-		name    string
-		fields  fields
-		args    args
-		want    *apb.DeleteLinkResponse
-		wantErr bool
+		name             string
+		fields           fields
+		args             args
+		want             *apb.DeleteLinkResponse
+		wantErr          bool
+		validationErrors []*validate.Violation
 	}{
 		{
-			name: "should add a new link",
+			name: "should delete an existing link",
 			fields: fields{
 				ports: []ports.Port{getTestSourcePort(), getTestTargetPort()},
 				nodes: []nodes.Node{getTestSourceNode(), getTestTargetNode()},
@@ -413,6 +444,28 @@ func TestTopology_DeleteLink(t *testing.T) {
 			want:    &apb.DeleteLinkResponse{},
 			wantErr: false,
 		},
+		{
+			name: "should error due to missing link id",
+			fields: fields{
+				ports: []ports.Port{getTestSourcePort(), getTestTargetPort()},
+				nodes: []nodes.Node{getTestSourceNode(), getTestTargetNode()},
+				links: []links.Link{getTestLinkInternal()},
+			},
+			args: args{
+				ctx: context.TODO(),
+				request: &apb.DeleteLinkRequest{
+					Id: "",
+				},
+			},
+			want:    &apb.DeleteLinkResponse{},
+			wantErr: true,
+			validationErrors: []*validate.Violation{
+				{
+					FieldPath:    "id",
+					ConstraintId: "required",
+					Message:      "value is required",
+				}},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -423,11 +476,18 @@ func TestTopology_DeleteLink(t *testing.T) {
 				return
 			}
 
+			if tt.wantErr {
+				assertValidationErrors(t, err, tt.validationErrors)
+				// There is no need to fetch again if we are expecting a validation error.
+				return
+			}
+
 			gotAfterDelete, err := tr.GetTopology(tt.args.ctx, &apb.GetTopologyRequest{})
 			if (err != nil) != tt.wantErr {
 				t.Errorf("Topology.GetTopology() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
+
 			if !reflect.DeepEqual(gotAfterDelete.Toplogy, &apb.Topology{}) {
 				t.Errorf("Topology.GetTopology() = %v, want %v", got, tt.want)
 			}