Bugfix: FilterSpecByPaths should not remove already unused definitions

 Please enter the commit message for your changes. Lines starting

pkg/aggregator/aggregator.go

@@ -154,8 +154,28 @@ func (s *referenceWalker) Start() {
 	}
 }
 
-// FilterSpecByPaths remove unnecessary paths and unused definitions.
+// usedDefinitionForSpec returns a map with all used definition in the provided spec as keys and true as values.
+func usedDefinitionForSpec(sp *spec.Swagger) map[string]bool {
+	usedDefinitions := map[string]bool{}
+	walkOnAllReferences(func(ref spec.Ref) spec.Ref {
+		if refStr := ref.String(); refStr != "" && strings.HasPrefix(refStr, definitionPrefix) {
+			usedDefinitions[refStr[len(definitionPrefix):]] = true
+		}
+		return ref
+	}, sp)
+	return usedDefinitions
+}
+
+// FilterSpecByPaths removes unnecessary paths and definitions used by those paths.
+// i.e. if a Path removed by this function, all definition used by it and not used
+// anywhere else will also be removed.
 func FilterSpecByPaths(sp *spec.Swagger, keepPathPrefixes []string) {
+	// Walk all references to find all used definitions. This function
+	// want to only deal with unused definitions resulted from filtering paths.
+	// Thus a definition will be removed only if it has been used before but
+	// it is unused because of a path prune.
+	initialUsedDefinitions := usedDefinitionForSpec(sp)
+
 	// First remove unwanted paths
 	prefixes := util.NewTrie(keepPathPrefixes)
 	orgPaths := sp.Paths
@@ -174,34 +194,24 @@ func FilterSpecByPaths(sp *spec.Swagger, keepPathPrefixes []string) {
 	}
 
 	// Walk all references to find all definition references.
-	usedDefinitions := map[string]bool{}
-
-	walkOnAllReferences(func(ref spec.Ref) spec.Ref {
-		if ref.String() != "" {
-			refStr := ref.String()
-			if strings.HasPrefix(refStr, definitionPrefix) {
-				usedDefinitions[refStr[len(definitionPrefix):]] = true
-			}
-		}
-		return ref
-	}, sp)
+	usedDefinitions := usedDefinitionForSpec(sp)
 
 	// Remove unused definitions
 	orgDefinitions := sp.Definitions
 	sp.Definitions = spec.Definitions{}
 	for k, v := range orgDefinitions {
-		if usedDefinitions[k] {
+		if usedDefinitions[k] || !initialUsedDefinitions[k] {
 			sp.Definitions[k] = v
 		}
 	}
 }
 
 func renameDefinition(s *spec.Swagger, old, new string) {
-	old_ref := definitionPrefix + old
-	new_ref := definitionPrefix + new
+	oldRef := definitionPrefix + old
+	newRef := definitionPrefix + new
 	walkOnAllReferences(func(ref spec.Ref) spec.Ref {
-		if ref.String() == old_ref {
-			return spec.MustCreateRef(new_ref)
+		if ref.String() == oldRef {
+			return spec.MustCreateRef(newRef)
 		}
 		return ref
 	}, s)
@@ -228,7 +238,6 @@ func MergeSpecs(dest, source *spec.Swagger) error {
 }
 
 func mergeSpecs(dest, source *spec.Swagger, renameModelConflicts, ignorePathConflicts bool) (err error) {
-
 	specCloned := false
 	if ignorePathConflicts {
 		keepPaths := []string{}

pkg/aggregator/aggregator_test.go

@@ -136,6 +136,109 @@ definitions:
 	assert.Equal(DebugSpec{spec1_filtered}, DebugSpec{spec1})
 }
 
+func TestFilterSpecsWithUnusedDefinitions(t *testing.T) {
+	var spec1, spec1Filtered *spec.Swagger
+	yaml.Unmarshal([]byte(`
+swagger: "2.0"
+paths:
+  /test:
+    post:
+      tags:
+      - "test"
+      summary: "Test API"
+      operationId: "addTest"
+      parameters:
+      - in: "body"
+        name: "body"
+        description: "test object"
+        required: true
+        schema:
+          $ref: "#/definitions/Test"
+      responses:
+        405:
+          description: "Invalid input"
+          $ref: "#/definitions/InvalidInput"
+  /othertest:
+    post:
+      tags:
+      - "test2"
+      summary: "Test2 API"
+      operationId: "addTest2"
+      consumes:
+      - "application/json"
+      produces:
+      - "application/xml"
+      parameters:
+      - in: "body"
+        name: "body"
+        description: "test2 object"
+        required: true
+        schema:
+          $ref: "#/definitions/Test2"
+definitions:
+  Test:
+    type: "object"
+    properties:
+      id:
+        type: "integer"
+        format: "int64"
+      status:
+        type: "string"
+        description: "Status"
+  InvalidInput:
+    type: "string"
+    format: "string"
+  Test2:
+    type: "object"
+    properties:
+      other:
+        $ref: "#/definitions/Other"
+  Other:
+    type: "string"
+  Unused:
+    type: "object"
+`), &spec1)
+	yaml.Unmarshal([]byte(`
+swagger: "2.0"
+paths:
+  /test:
+    post:
+      tags:
+      - "test"
+      summary: "Test API"
+      operationId: "addTest"
+      parameters:
+      - in: "body"
+        name: "body"
+        description: "test object"
+        required: true
+        schema:
+          $ref: "#/definitions/Test"
+      responses:
+        405:
+          description: "Invalid input"
+          $ref: "#/definitions/InvalidInput"
+definitions:
+  Test:
+    type: "object"
+    properties:
+      id:
+        type: "integer"
+        format: "int64"
+      status:
+        type: "string"
+        description: "Status"
+  InvalidInput:
+    type: "string"
+    format: "string"
+  Unused:
+    type: "object"
+`), &spec1Filtered)
+	assert := assert.New(t)
+	FilterSpecByPaths(spec1, []string{"/test"})
+	assert.Equal(DebugSpec{spec1Filtered}, DebugSpec{spec1})
+}
+
 func TestMergeSpecsSimple(t *testing.T) {
 	var spec1, spec2, expected *spec.Swagger
 	yaml.Unmarshal([]byte(`