Federation: improve entities handling on sdl merge (#395)

* [TT-4815] Remove duplicated scalars from federated schemas

[changelog]
internal: Duplicated scalars among merged schemas will be removed so only one of that scalar name will exist within the federated schema.

* [TT-4815] Responded to PR feedback

[changelog]
internal: Changed magic number to constant; renamed variable; removed unnecessary code.

* [TT-5116] Duplicate enums and unions are merged into a single enum or union respectively

[changelog]
internal: Duplicate enums and unions are merged into a single enum or union respectively.

* [TT-5116] Responding to PR feedback

[changelog]
internal: Reduced some code duplication through interfaces.

* [TT-5116] Some renaming and clean up.

[changelog]
internal: Renamed and clean up some methods; proposed some efficiency solutions.

* [TT-5116] Duplicate fieldless value types are removed through single visitor.

[changelog]
internal: Duplicate enums, scalars and unions are merged through the same generic visitor.

* [TT-5116] Return an error if duplicate value types are not identically composed.

[changelog]
internal: Added error handling for when duplicate values are not identically composed.

* [TT-5116] Added sldmerge_tests

[changelog]
internal: Added new tests; improved test formatting; improved some naming.

* [TT-5116] Removed unnecessary code
[changelog]
internal: Removed unnecessary code; tidied code up.

* [TT-5116] Minor additions to sdlmerge_test
[changelog]
internal: Capitalised enum values; added one more type to unions.

* [TT-5117] Implemented removal of duplicate interfaces

[changelog]
internal: Implemented removal of duplicate interfaces.

(cherry picked from commit c2388b4)

* [TT-5117] Implemented duplicate Input/Object/Interface removal
[changelog]
internal: Implemented duplicate Input/Object/Interface removal.

(cherry picked from commit edc4f3d)

* [TT-5117] Removed duplicate code
[changelog]
internal: Removed duplicate code.

(cherry picked from commit d12e057)

* [TT-5117] Changed receiver to pointer.
[changelog]
internal: Changed receiver to pointer.

(cherry picked from commit 5ab441c)

* [TT-5117] Removed unused interface.
[changelog]
internal: Removed unused interface.

(cherry picked from commit cdd0b14)

* [TT-5117] Re-added interface tests
[changelog]
internal: Re-added interface tests; improved test naming.

(cherry picked from commit 06e0c87)

* [TT-5117] Corrected comparison of nested field values.
[changelog]
internal: Field values are deeply compared; added more tests.

(cherry picked from commit a68d94d)

* [TT-5117] Expanded fielded and sdlmerge test suites.
[changelog]
internal: Expanded fielded and sdlmerge test suites.

(cherry picked from commit de97afa)

* [TT-5199] Subgraphs are normalized before a merge is attempted.
[changelog]
internal: Subgraphs are normalized before merge; added missing extension handlers.

(cherry picked from commit f4e821c)

* [TT-5199] Removed redundant boolean.
[changelog]
internal: Removed redundant boolean due to early return.

(cherry picked from commit 560e9e1)

* [TT-5199] Extracted pre-merge subgraph normalization into separate method.
[changelog]
internal: Extracted pre-merge subgraph normalization into separate method.

(cherry picked from commit 83295b3)

* [TT-5117] Entities cannot be shared types

[changelog]
internal: Added logic to produce error if duplicate entity exists; renamed value type to shared type.

(cherry picked from commit 79783e8)

* [TT-5117] Handled an edge case where entity is last item in the schema

[changelog]
internal: Added logic to still error if the entity is the last item in the schema.

(cherry picked from commit 091ded1)

* [TT-5117] Inputs cannot be entities

[changelog]
internal: Removed logic to handle input entities.

(cherry picked from commit d8a9fa6)

* [TT-5279] Shared type, extension orphan and validation clean up

[changelog]
internal: Shared types cannot be extended; added unique union member validation; unresolved extension orphans in the supergraph return an error

(cherry picked from commit 9295cc4)

* [TT-5279] Subgraphs with empty type bodies are invalidated before extension could occur

[changelog]
internal: Added visitor to return an error if a type has an empty body.

(cherry picked from commit 6820fd5)

* [TT-5279] Moved empty type body validation to astvalidation

[changelog]
internal: Moved visitor to astvalidation. Added validation for type extensions. Handled some edge cases.

(cherry picked from commit a8b8544)

* [TT-5279] Handled edge cases around pre-validation of subgraphs

[changelog]
internal: Added check for root type; added logic to ignore private fields; removed duplicated line in rule.

(cherry picked from commit e96a455)

* [TT-5279] Final clean up

[changelog]
internal: Removed unnecessary visitor; changed tests to new expected outcome

(cherry picked from commit 96d3cc4)

* [TT-5279] Changed reused string to constant

[changelog]
internal: Changed reused parse error to string constant.

(cherry picked from commit b271c75)

* [TT-5279] Early return for root types when checking whether an object is empty

[changelog]
internal: Changed check for root type to come first so unnecessary code is not performed.

(cherry picked from commit 4430be0)

* [TT-5279] Responding to PR feedback

[changelog]
internal: Added vars to prevent repeated slice creation; added helper function to determine whether type only contains reserved fields.

(cherry picked from commit d4210ec)

* [TT-5279] Use strings to create byte slice vars

[changelog]
internal: Changed explicit byte slice creation to string conversion.

(cherry picked from commit 55244b8)

* [TT-5279] Removed cached struct fields

[changelog]
internal: Removed variables that cached struct fields.

(cherry picked from commit 3237472)

* [TT-5460] Handling of entities PoC

[changelog]
internal: Added handling for extension of entities.

(cherry picked from commit c0ac6e5)

* [TT-5460] Fixed gosimple complaint

[changelog]
internal: Removed explicit length for empty map.

(cherry picked from commit 163b185)

* [TT-5460] Refactors based on feedback

[changelog]
internal: Refactored implementation of entity handling; moved entity handling and shared type handling into own files.

(cherry picked from commit 59ce182)

* [TT-5460] Minor formatting changes.

[changelog]
internal: Changed some formatting; exported and reused existing constant.

(cherry picked from commit 2e2c521)

* [TT-5460] Reduced code duplication and cognitive complexity

[changelog]
internal: Reduced code duplication; reduced cognitive complexity of external directive validation method.

(cherry picked from commit 6cde0ac)

* [TT-5460] Further reduced code duplication

[changelog]
internal: Moved more generic isEntity method to entityValidator.

(cherry picked from commit 97a6a5f)

* [TT-5460] SOm refactoring of entity handling.

[changelog]
internal: Renamed some errors; added logic for edge cases.

(cherry picked from commit bcceac4)

* [TT-5460] Removed most validation of entities

[changelog]
internal: Removed most validation of entities.

(cherry picked from commit 6059c34)

* [TT-5460] Responded to PR feedback

[changelog]
internal: Changed placeholder bool to empty struct in entity set; refactored handling of duplicated entities; removed unnecessary checks that are handled elsewhere.

(cherry picked from commit 9e715cf)

* [TT-5460] Responded to more PR feedback

[changelog]
internal: Added immediate return when producing an error; added entitySet type and pass that rather than normalizer.

(cherry picked from commit f7dd1be)

* [TT-5460] Final PR feedback changes

[changelog]
internal: Renamed isTypeEntity; changed logic so document is only changed if the extension is valid; separated exported and unexported const blocks in Plan.

(cherry picked from commit 16baec3)

* [TT-5460] Removed unnecessary variable.

[changelog]
internal: Removed unnecessary variable.

(cherry picked from commit 5389d86)

* [federation-changes-for-upstream] Changed import paths

[changelog]
internal: Changed import paths.

Co-authored-by: David Stutt <david.s@tyk.io>

pkg/ast/ast_object_type_definition_test.go

@@ -35,24 +35,24 @@ func TestDocument_RemoveObjectTypeDefinition(t *testing.T) {
 
 	t.Run("remove query type", func(t *testing.T) {
 		doc := prepareDoc()
-		doc.RemoveObjectTypeDefinition([]byte("Query"))
+		doc.RemoveObjectTypeDefinition(ast.DefaultQueryTypeName)
 		docStr, _ := astprinter.PrintString(doc, nil)
 		assert.Equal(t, "type Mutation {mutationName: String} type Country {code: String} interface Model {id: String}", docStr)
 	})
 
 	t.Run("remove query and mutations types", func(t *testing.T) {
 		doc := prepareDoc()
-		doc.RemoveObjectTypeDefinition([]byte("Query"))
-		doc.RemoveObjectTypeDefinition([]byte("Mutation"))
+		doc.RemoveObjectTypeDefinition(ast.DefaultQueryTypeName)
+		doc.RemoveObjectTypeDefinition(ast.DefaultMutationTypeName)
 
 		docStr, _ := astprinter.PrintString(doc, nil)
 		assert.Equal(t, "type Country {code: String} interface Model {id: String}", docStr)
 	})
 
 	t.Run("remove all types", func(t *testing.T) {
 		doc := prepareDoc()
-		doc.RemoveObjectTypeDefinition([]byte("Query"))
-		doc.RemoveObjectTypeDefinition([]byte("Mutation"))
+		doc.RemoveObjectTypeDefinition(ast.DefaultQueryTypeName)
+		doc.RemoveObjectTypeDefinition(ast.DefaultMutationTypeName)
 		doc.RemoveObjectTypeDefinition([]byte("Country"))
 
 		docStr, _ := astprinter.PrintString(doc, nil)

pkg/ast/ast_root_operation_type_definition.go

@@ -1,9 +1,14 @@
 package ast
 
 import (
+	"bytes"
 	"github.com/wundergraph/graphql-go-tools/pkg/lexer/position"
 )
 
+var DefaultQueryTypeName = []byte("Query")
+var DefaultMutationTypeName = []byte("Mutation")
+var DefaultSubscriptionTypeName = []byte("Subscription")
+
 type RootOperationTypeDefinitionList struct {
 	LBrace position.Position // {
 	Refs   []int             // RootOperationTypeDefinition
@@ -51,11 +56,11 @@ func (d *Document) RootOperationTypeDefinitionIsLastInSchemaDefinition(ref int,
 func (d *Document) CreateRootOperationTypeDefinition(operationType OperationType, rootNodeRef int) (ref int) {
 	switch operationType {
 	case OperationTypeQuery:
-		d.Index.QueryTypeName = []byte("Query")
+		d.Index.QueryTypeName = DefaultQueryTypeName
 	case OperationTypeMutation:
-		d.Index.MutationTypeName = []byte("Mutation")
+		d.Index.MutationTypeName = DefaultMutationTypeName
 	case OperationTypeSubscription:
-		d.Index.SubscriptionTypeName = []byte("Subscription")
+		d.Index.SubscriptionTypeName = DefaultSubscriptionTypeName
 	default:
 		return
 	}
@@ -137,3 +142,7 @@ func (d *Document) ReplaceRootOperationTypeDefinition(name string, operationType
 	ref = d.ImportRootOperationTypeDefinition(name, operationType)
 	return ref, true
 }
+
+func IsRootType(nameBytes []byte) bool {
+	return bytes.Equal(DefaultQueryTypeName, nameBytes) || bytes.Equal(DefaultMutationTypeName, nameBytes) || bytes.Equal(DefaultSubscriptionTypeName, nameBytes)
+}

pkg/ast/ast_test.go

@@ -400,7 +400,7 @@ func TestDocument_NodeByName(t *testing.T) {
 
 		t.Run("when node name is Query", func(t *testing.T) {
 			t.Run("NodeByName", func(t *testing.T) {
-				node, exists := doc.NodeByName([]byte("Query"))
+				node, exists := doc.NodeByName(ast.DefaultQueryTypeName)
 				assert.Equal(t, ast.NodeKindObjectTypeDefinition, node.Kind)
 				assert.True(t, exists)
 			})

pkg/astnormalization/definition_normalization.go

@@ -41,6 +41,26 @@ func (o *DefinitionNormalizer) setupWalkers() {
 	o.walker = &walker
 }
 
+func NewSubgraphDefinitionNormalizer() *DefinitionNormalizer {
+	normalizer := &DefinitionNormalizer{}
+	normalizer.setupSubgraphWalkers()
+	return normalizer
+}
+
+func (o *DefinitionNormalizer) setupSubgraphWalkers() {
+	walker := astvisitor.NewWalker(48)
+
+	extendObjectTypeDefinitionKeepingOrphans(&walker)
+	extendInputObjectTypeDefinitionKeepingOrphans(&walker)
+	extendEnumTypeDefinitionKeepingOrphans(&walker)
+	extendInterfaceTypeDefinitionKeepingOrphans(&walker)
+	extendScalarTypeDefinitionKeepingOrphans(&walker)
+	extendUnionTypeDefinitionKeepingOrphans(&walker)
+	removeMergedTypeExtensions(&walker)
+
+	o.walker = &walker
+}
+
 // NormalizeDefinition applies all registered rules to the AST
 func (o *DefinitionNormalizer) NormalizeDefinition(definition *ast.Document, report *operationreport.Report) {
 	o.walker.Walk(definition, nil, report)

pkg/astnormalization/definition_normalization_test.go

@@ -78,8 +78,8 @@ func TestNormalizeDefinition(t *testing.T) {
 				lat: Float 
 				lon: Float
 				planet: Planet
-			}`,
-		)
+			}
+		`)
 	})
 
 	t.Run("removes type extension and includes interfaces when type already has implements interface", func(t *testing.T) {
@@ -100,7 +100,8 @@ func TestNormalizeDefinition(t *testing.T) {
 			
 			interface Entity {
 				id: ID
-			}`, `
+			}
+		`, `
 			schema { query: Query }
 			
 			type User implements Named & Entity {
@@ -114,8 +115,8 @@ func TestNormalizeDefinition(t *testing.T) {
 			
 			interface Entity {
 				id: ID
-			}`,
-		)
+			}
+		`)
 	})
 
 	t.Run("removes extensions and creates missing schema and root operation types", func(t *testing.T) {
@@ -133,7 +134,129 @@ func TestNormalizeDefinition(t *testing.T) {
 			}
 			type Subscription {
 				textCounter: String
-			}`,
-		)
+			}
+		`)
+	})
+}
+
+func TestNormalizeSubgraphDefinition(t *testing.T) {
+	run := func(t *testing.T, definition, expectedOutput string) {
+		t.Helper()
+
+		definitionDocument := unsafeparser.ParseGraphqlDocumentString(definition)
+		expectedOutputDocument := unsafeparser.ParseGraphqlDocumentString(expectedOutput)
+
+		report := operationreport.Report{}
+		normalizer := NewSubgraphDefinitionNormalizer()
+		normalizer.NormalizeDefinition(&definitionDocument, &report)
+
+		if report.HasErrors() {
+			t.Fatal(report.Error())
+		}
+
+		got := mustString(astprinter.PrintString(&definitionDocument, nil))
+		want := mustString(astprinter.PrintString(&expectedOutputDocument, nil))
+
+		assert.Equal(t, want, got)
+	}
+
+	t.Run("Extension orphans are not deleted", func(t *testing.T) {
+		run(t, `
+			extend type Rival {
+				version: Version!
+			}
+
+			enum Badge {
+				BOULDER
+				SOUL
+			}
+
+			extend enum Version {
+				SILVER
+			}
+
+			extend input Deposit {
+				quantity: Int!
+			}
+
+			extend interface GymLeader {
+				badge: Badge!
+			}
+			
+			type Pokemon {
+				name: String!
+			}
+			
+			extend interface Trainer {
+				age: Int!
+			}
+
+			union Types = Water | Fire
+	
+			extend input Move {
+				name: String
+			}
+
+			input Deposit {
+				item: String!
+			}
+			
+			extend enum Badge {
+				EARTH
+			}
+			
+			extend union Berry = Oran
+	
+			extend type Pokemon {
+				types: Types!
+			}
+			
+			extend union Types = Grass
+			
+			interface Trainer {
+				name: String!
+			}
+		`, `
+			extend type Rival {
+				version: Version!
+			}
+
+			enum Badge {
+				BOULDER
+				SOUL
+				EARTH
+			}
+
+			extend enum Version {
+				SILVER
+			}
+
+			extend interface GymLeader {
+				badge: Badge!
+			}
+
+			type Pokemon {
+				name: String!
+				types: Types!
+			}
+
+			union Types = Water | Fire | Grass
+
+			extend input Move {
+				name: String
+			}
+
+			input Deposit {
+				item: String!
+				quantity: Int!
+			}
+			
+			extend union Berry = Oran
+
+			interface Trainer {
+				name: String!
+				age: Int!
+			}
+		`)
 	})
 }

pkg/astnormalization/enum_type_extending.go

@@ -13,12 +13,22 @@ func extendEnumTypeDefinition(walker *astvisitor.Walker) {
 	walker.RegisterEnterEnumTypeExtensionVisitor(&visitor)
 }
 
+func extendEnumTypeDefinitionKeepingOrphans(walker *astvisitor.Walker) {
+	visitor := extendEnumTypeDefinitionVisitor{
+		Walker:               walker,
+		keepExtensionOrphans: true,
+	}
+	walker.RegisterEnterDocumentVisitor(&visitor)
+	walker.RegisterEnterEnumTypeExtensionVisitor(&visitor)
+}
+
 type extendEnumTypeDefinitionVisitor struct {
 	*astvisitor.Walker
-	operation *ast.Document
+	operation            *ast.Document
+	keepExtensionOrphans bool
 }
 
-func (e *extendEnumTypeDefinitionVisitor) EnterDocument(operation, definition *ast.Document) {
+func (e *extendEnumTypeDefinitionVisitor) EnterDocument(operation, _ *ast.Document) {
 	e.operation = operation
 }
 
@@ -36,5 +46,9 @@ func (e *extendEnumTypeDefinitionVisitor) EnterEnumTypeExtension(ref int) {
 		return
 	}
 
+	if e.keepExtensionOrphans {
+		return
+	}
+
 	e.operation.ImportAndExtendEnumTypeDefinitionByEnumTypeExtension(ref)
 }

pkg/astnormalization/input_object_type_extending.go

@@ -13,17 +13,26 @@ func extendInputObjectTypeDefinition(walker *astvisitor.Walker) {
 	walker.RegisterEnterInputObjectTypeExtensionVisitor(&visitor)
 }
 
+func extendInputObjectTypeDefinitionKeepingOrphans(walker *astvisitor.Walker) {
+	visitor := extendInputObjectTypeDefinitionVisitor{
+		Walker:               walker,
+		keepExtensionOrphans: true,
+	}
+	walker.RegisterEnterDocumentVisitor(&visitor)
+	walker.RegisterEnterInputObjectTypeExtensionVisitor(&visitor)
+}
+
 type extendInputObjectTypeDefinitionVisitor struct {
 	*astvisitor.Walker
-	operation *ast.Document
+	operation            *ast.Document
+	keepExtensionOrphans bool
 }
 
-func (e *extendInputObjectTypeDefinitionVisitor) EnterDocument(operation, definition *ast.Document) {
+func (e *extendInputObjectTypeDefinitionVisitor) EnterDocument(operation, _ *ast.Document) {
 	e.operation = operation
 }
 
 func (e *extendInputObjectTypeDefinitionVisitor) EnterInputObjectTypeExtension(ref int) {
-
 	nodes, exists := e.operation.Index.NodesByNameBytes(e.operation.InputObjectTypeExtensionNameBytes(ref))
 	if !exists {
 		return
@@ -37,5 +46,9 @@ func (e *extendInputObjectTypeDefinitionVisitor) EnterInputObjectTypeExtension(r
 		return
 	}
 
+	if e.keepExtensionOrphans {
+		return
+	}
+
 	e.operation.ImportAndExtendInputObjectTypeDefinitionByInputObjectTypeExtension(ref)
 }

pkg/astnormalization/interface_type_extending.go

@@ -13,17 +13,26 @@ func extendInterfaceTypeDefinition(walker *astvisitor.Walker) {
 	walker.RegisterEnterInterfaceTypeExtensionVisitor(&visitor)
 }
 
+func extendInterfaceTypeDefinitionKeepingOrphans(walker *astvisitor.Walker) {
+	visitor := extendInterfaceTypeDefinitionVisitor{
+		Walker:               walker,
+		keepExtensionOrphans: true,
+	}
+	walker.RegisterEnterDocumentVisitor(&visitor)
+	walker.RegisterEnterInterfaceTypeExtensionVisitor(&visitor)
+}
+
 type extendInterfaceTypeDefinitionVisitor struct {
 	*astvisitor.Walker
-	operation *ast.Document
+	operation            *ast.Document
+	keepExtensionOrphans bool
 }
 
-func (e *extendInterfaceTypeDefinitionVisitor) EnterDocument(operation, definition *ast.Document) {
+func (e *extendInterfaceTypeDefinitionVisitor) EnterDocument(operation, _ *ast.Document) {
 	e.operation = operation
 }
 
 func (e *extendInterfaceTypeDefinitionVisitor) EnterInterfaceTypeExtension(ref int) {
-
 	nodes, exists := e.operation.Index.NodesByNameBytes(e.operation.InterfaceTypeExtensionNameBytes(ref))
 	if !exists {
 		return
@@ -37,5 +46,9 @@ func (e *extendInterfaceTypeDefinitionVisitor) EnterInterfaceTypeExtension(ref i
 		return
 	}
 
+	if e.keepExtensionOrphans {
+		return
+	}
+
 	e.operation.ImportAndExtendInterfaceTypeDefinitionByInterfaceTypeExtension(ref)
 }