[TT-5460] Reduced code duplication and cognitive complexity

[changelog] internal: Reduced code duplication; reduced cognitive complexity of external directive validation method.
wundergraph · Jun 1, 2022 · 6cde0ac · 6cde0ac
1 parent 2e2c521
commit 6cde0ac
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 30 deletions.
diff --git a/pkg/federation/sdlmerge/collect_valid_entities.go b/pkg/federation/sdlmerge/collect_valid_entities.go
@@ -35,7 +35,6 @@ func (c *collectValidEntitiesVisitor) EnterInterfaceTypeDefinition(ref int) {
 	if !interfaceType.HasDirectives {
 		return
 	}
-
 	name := c.document.InterfaceTypeDefinitionNameString(ref)
 	c.resolveEntity(name, interfaceType.Directives.Refs, interfaceType.FieldsDefinition.Refs)
 }
@@ -45,7 +44,6 @@ func (c *collectValidEntitiesVisitor) EnterObjectTypeDefinition(ref int) {
 	if !objectType.HasDirectives {
 		return
 	}
-
 	name := c.document.ObjectTypeDefinitionNameString(ref)
 	c.resolveEntity(name, objectType.Directives.Refs, objectType.FieldsDefinition.Refs)
 }
@@ -55,15 +53,13 @@ func (c *collectValidEntitiesVisitor) resolveEntity(name string, directiveRefs [
 	if _, exists := validator.entitySet[name]; exists {
 		c.Walker.StopWithExternalErr(operationreport.ErrEntitiesMustNotBeDuplicated(name))
 	}
-
 	primaryKeys, err := validator.getPrimaryKeys(name, directiveRefs, false)
 	if err != nil {
 		c.Walker.StopWithExternalErr(*err)
 	}
 	if primaryKeys == nil {
 		return
 	}
-
 	validator.entitySet[name] = primaryKeys
 	validator.validatePrimaryKeyReferences(name, fieldRefs)
 }
diff --git a/pkg/federation/sdlmerge/entity_validator.go b/pkg/federation/sdlmerge/entity_validator.go
@@ -103,27 +103,29 @@ func (e *entityValidator) validateExternalPrimaryKeys(name string, primaryKeys p
 		if !isPrimaryKey {
 			continue
 		}
-		field := e.document.FieldDefinitions[fieldRef]
-		hasExternalDirective := false
-		for _, directiveRef := range field.Directives.Refs {
-			if e.document.DirectiveNameString(directiveRef) != plan.FederationExternalDirectiveName {
-				continue
-			}
-			hasExternalDirective = true
-			if !isExternalDirectiveResolved {
-				primaryKeys[fieldName] = true
-				fieldReferences -= 1
-			}
-			if fieldReferences == 0 {
-				return nil
-			}
-			break
+		if err := e.validateExternalField(fieldRef, name, fieldName, isExternalDirectiveResolved, primaryKeys); err != nil {
+			return err
 		}
-		if !hasExternalDirective {
-			err := operationreport.ErrEntityExtensionPrimaryKeyFieldReferenceMustHaveExternalDirective(name)
-			return &err
+		fieldReferences -= 1
+		if fieldReferences == 0 {
+			return nil
 		}
 	}
 	err := operationreport.ErrEntityExtensionPrimaryKeyMustExistAsField(name)
 	return &err
 }
+
+func (e *entityValidator) validateExternalField(fieldRef int, name, fieldName string, isExternalDirectiveResolved bool, primaryKeys primaryKeySet) *operationreport.ExternalError {
+	field := e.document.FieldDefinitions[fieldRef]
+	for _, directiveRef := range field.Directives.Refs {
+		if e.document.DirectiveNameString(directiveRef) != plan.FederationExternalDirectiveName {
+			continue
+		}
+		if !isExternalDirectiveResolved {
+			primaryKeys[fieldName] = true
+		}
+		return nil
+	}
+	err := operationreport.ErrEntityExtensionPrimaryKeyFieldReferenceMustHaveExternalDirective(name)
+	return &err
+}
diff --git a/pkg/federation/sdlmerge/interface_type_extending.go b/pkg/federation/sdlmerge/interface_type_extending.go
@@ -42,10 +42,7 @@ func (e *extendInterfaceTypeDefinitionVisitor) EnterInterfaceTypeExtension(ref i
 			continue
 		}
 		if hasExtended {
-			if isEntity {
-				e.Walker.StopWithExternalErr(operationreport.ErrEntitiesMustNotBeDuplicated(string(nameBytes)))
-			}
-			e.Walker.StopWithExternalErr(operationreport.ErrSharedTypesMustNotBeExtended(e.document.InterfaceTypeExtensionNameString(ref)))
+			e.Walker.StopWithExternalErr(*getMultipleExtensionError(isEntity, nameBytes))
 		}
 		isEntity = e.isEntity(ref, nameBytes)
 		e.document.ExtendInterfaceTypeDefinitionByInterfaceTypeExtension(node.Ref, ref)

diff --git a/pkg/federation/sdlmerge/object_type_extending.go b/pkg/federation/sdlmerge/object_type_extending.go
@@ -43,10 +43,7 @@ func (e *extendObjectTypeDefinitionVisitor) EnterObjectTypeExtension(ref int) {
 			continue
 		}
 		if hasExtended {
-			if isEntity {
-				e.Walker.StopWithExternalErr(operationreport.ErrEntitiesMustNotBeDuplicated(string(nameBytes)))
-			}
-			e.Walker.StopWithExternalErr(operationreport.ErrSharedTypesMustNotBeExtended(string(nameBytes)))
+			e.Walker.StopWithExternalErr(*getMultipleExtensionError(isEntity, nameBytes))
 		}
 		isEntity = e.isEntity(ref, nameBytes)
 		e.document.ExtendObjectTypeDefinitionByObjectTypeExtension(nodes[i].Ref, ref)

diff --git a/pkg/federation/sdlmerge/sdlmerge.go b/pkg/federation/sdlmerge/sdlmerge.go
@@ -162,3 +162,12 @@ func (m *normalizer) normalize(operation *ast.Document) error {
 
 	return nil
 }
+
+func getMultipleExtensionError(isEntity bool, nameBytes []byte) *operationreport.ExternalError {
+	if isEntity {
+		err := operationreport.ErrEntitiesMustNotBeDuplicated(string(nameBytes))
+		return &err
+	}
+	err := operationreport.ErrSharedTypesMustNotBeExtended(string(nameBytes))
+	return &err
+}