Fixes #1129 - allOf fix for $$ref values

src/specmap/lib/all-of.js

@@ -17,7 +17,16 @@ export default {
     const parent = fullPath.slice(0, -1)
     let alreadyAddError = false
 
-    return [specmap.replace(parent, {})].concat(val.map((toMerge, index) => {
+    // Find the original definition from the `patch.value` object
+    // Remove the `allOf` property so it doesn't get added to the result of the `allOf` plugin
+    let originalDefinitionObj = patch.value
+    parent.forEach((part) => {
+      originalDefinitionObj = originalDefinitionObj[part]
+    })
+    originalDefinitionObj = Object.assign({}, originalDefinitionObj)
+    delete originalDefinitionObj.allOf
+
+    const allOfPatches = [specmap.replace(parent, {})].concat(val.map((toMerge, index) => {
       if (!specmap.isObject(toMerge)) {
         if (alreadyAddError) {
           return null
@@ -31,5 +40,16 @@ export default {
 
       return specmap.mergeDeep(parent, toMerge)
     }))
+
+    // Merge back the values from the original definition
+    allOfPatches.push(specmap.mergeDeep(parent, originalDefinitionObj))
+
+    // If there was not an original $$ref value, make sure to remove
+    // any $$ref value that may exist from the result of `allOf` merges
+    if (!originalDefinitionObj.$$ref) {
+      allOfPatches.push(specmap.remove([].concat(parent, '$$ref')))
+    }
+
+    return allOfPatches
   }
 }

test/specmap/all-of.js

@@ -56,6 +56,124 @@ describe('allOf', function () {
         spec: {
           one: {baz: 4},
           two: 2,
+          bar: {baz: 4}
+        }
+      })
+    })
+  })
+
+  it('should not overwrite properties that are already present', function () {
+    return mapSpec({
+      spec: {
+        original: 'yes',
+        allOf: [
+          {
+            original: 'no',
+            notOriginal: 'yes'
+          }
+        ]
+      },
+      plugins: [plugins.refs, plugins.allOf]
+    }).then((res) => {
+      expect(res).toEqual({
+        errors: [],
+        spec: {
+          original: 'yes',
+          notOriginal: 'yes'
+        }
+      })
+    })
+  })
+
+  it('should set $$ref values', function () {
+    return mapSpec({
+      allowMetaPatches: true,
+      spec: {
+        Pet: {
+          type: 'object',
+          properties: {
+            name: {
+              type: 'string'
+            }
+          }
+        },
+        Cat: {
+          allOf: [
+            {$ref: '#/Pet'},
+            {
+              type: 'object',
+              properties: {
+                meow: {
+                  type: 'string'
+                }
+              }
+            }
+          ]
+        },
+        Animal: {
+          type: 'object',
+          properties: {
+            pet: {
+              $ref: '#/Pet'
+            },
+            cat: {
+              $ref: '#/Cat'
+            }
+          }
+        }
+      },
+      plugins: [plugins.refs, plugins.allOf]
+    }).then((res) => {
+      expect(res).toEqual({
+        errors: [],
+        spec: {
+          Pet: {
+            $$ref: '#/Pet',
+            type: 'object',
+            properties: {
+              name: {
+                type: 'string'
+              }
+            }
+          },
+          Cat: {
+            $$ref: '#/Cat',
+            properties: {
+              meow: {
+                type: 'string'
+              },
+              name: {
+                type: 'string'
+              }
+            },
+            type: 'object'
+          },
+          Animal: {
+            type: 'object',
+            properties: {
+              pet: {
+                $$ref: '#/Pet',
+                properties: {
+                  name: {
+                    type: 'string'
+                  }
+                },
+                type: 'object'
+              },
+              cat: {
+                $$ref: '#/Cat',
+                properties: {
+                  meow: {
+                    type: 'string'
+                  },
+                  name: {
+                    type: 'string'
+                  }
+                },
+                type: 'object'
+              }
+            }
+          }
         }
       })
     })
@@ -218,8 +336,7 @@ describe('allOf', function () {
     })
   })
 
-  // TODO: this needs to get fixed
-  it.skip('should handle case, with an `allOf` referencing an `allOf` ', function () {
+  it('should handle case, with an `allOf` referencing an `allOf` ', function () {
     return mapSpec({
       plugins: [plugins.refs, plugins.allOf],
       showDebug: true,