Fix for cyclic schemas

lib/formattingTools.js

@@ -13,9 +13,13 @@ const i18n = require('es2015-i18n-tag').default;
 const s = require('./symbols');
 
 function gentitle(titles, type) {
+  if (!Array.isArray(titles)) {
+    return i18n`Untitled schema`;
+  }
   const [firsttitle] = titles;
   const lasttitle = [...titles].pop();
-  if (titles.length === 1) {
+
+  if (titles.length === 1 && firsttitle !== undefined) {
     return firsttitle;
   }
   if (lasttitle) {
@@ -24,6 +28,9 @@ function gentitle(titles, type) {
   if (typeof type === 'string') {
     return i18n`Untitled ${type} in ${firsttitle}`;
   }
+  if (firsttitle === undefined) {
+    return i18n`Untitled Schema`;
+  }
   return i18n`Untitled undefined type in ${firsttitle}`;
 }
 

lib/markdownBuilder.js

@@ -306,7 +306,8 @@ function build({
       tableCell(type(definition)),
       tableCell(text(required.indexOf(name) > -1 ? i18n`Required` : i18n`Optional`)),
       tableCell(nullable(definition)),
-      tableCell(link(`${definition[s.slug]}.md`, `${definition[s.id]}#${definition[s.pointer]}`, text(definition[s.titles][0] || i18n`Untitled schema`))),
+      tableCell(link(`${definition[s.slug]}.md`, `${definition[s.id]}#${definition[s.pointer]}`,
+        text(definition[s.titles] && definition[s.titles][0] ? definition[s.titles][0] : i18n`Untitled schema`))),
     ]);
   }
 
@@ -440,7 +441,7 @@ function build({
   function makedefinedinfact(definition) {
     return listItem(paragraph([
       text(i18n`defined in: `),
-      link(`${definition[s.slug]}.md`, `${definition[s.id]}#${definition[s.pointer]}`, text(definition[s.titles][0] || i18n`Untitled schema`)),
+      link(`${definition[s.slug]}.md`, `${definition[s.id]}#${definition[s.pointer]}`, text(definition[s.titles] && definition[s.titles][0] ? definition[s.titles][0] : i18n`Untitled schema`)),
     ]));
   }
 
@@ -702,7 +703,7 @@ function build({
     level = 2) {
     return [
       ...flist(flat(Object.entries(properties || {}).map(([name, definition]) => {
-        const description = definition[s.meta].longdescription || paragraph(text(i18n`no description`));
+        const description = definition[s.meta] && definition[s.meta].longdescription ? definition[s.meta].longdescription : paragraph(text(i18n`no description`));
 
         return [
           heading(level + 1, text(name)),

lib/traverseSchema.js

@@ -9,29 +9,25 @@
  * OF ANY KIND, either express or implied. See the License for the specific language
  * governing permissions and limitations under the License.
  */
-const { list } = require('ferrum');
 
-function sflat(arr) {
-  return arr.reduce((p, v) => {
-    if (Array.isArray(v)) {
-      return [...p, ...v];
-    }
-    return [...p, v];
-  }, []);
-}
+const { parent } = require('./symbols');
 
-/**
- * Traverses a (proxied) JSON schema. This is a less opinionated
- * and tighter traversal.
- * @param {*} schema
- */
-function traverseSchema(schema) {
-  if (Array.isArray(schema)) {
-    return sflat(schema.map(traverseSchema));
+function reducer({ seen, ids }, schema) {
+  if (schema
+    && schema[parent]
+    && (seen.has(schema) || (schema.$id && ids.indexOf(schema.$id) >= 0))) {
+    return { seen, ids };
+  } else if (Array.isArray(schema)) {
+    return schema.reduce(reducer, { seen, ids });
   } else if (schema && typeof schema === 'object') {
-    return sflat([schema, ...Object.values(schema).map(traverseSchema)]);
+    seen.add(schema);
+    if (schema.$id) {
+      ids.push(schema.$id);
+    }
+    return [...Object.values(schema)].reduce(reducer, { seen, ids });
   }
-  return [];
+
+  return { seen, ids };
 }
 
 /**
@@ -40,8 +36,7 @@ function traverseSchema(schema) {
  * @param {Schema} node
  */
 function traverse(schemas) {
-  return sflat(list(schemas).map(traverseSchema));
+  return Array.from(schemas.reduce(reducer, { seen: new Set(), ids: [] }).seen);
 }
 
 module.exports = traverse;
-module.exports.traverseSchema = traverseSchema;

test/cyclicSchemaIntegration.test.js

@@ -0,0 +1,71 @@
+/*
+ * Copyright 2019 Adobe. All rights reserved.
+ * This file is licensed to you under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License. You may obtain a copy
+ * of the License at http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under
+ * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
+ * OF ANY KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+/* eslint-env mocha */
+
+const path = require('path');
+const fs = require('fs-extra');
+const { loader } = require('../lib/schemaProxy');
+const { assertMarkdown } = require('./testUtils');
+const readme = require('../lib/readmeBuilder');
+const markdown = require('../lib/markdownBuilder');
+const traverse = require('../lib/traverseSchema');
+
+describe('Integration Test: Cyclic References', () => {
+  let one;
+  let two;
+  let three;
+
+  let proxiedone;
+  let proxiedtwo;
+  let proxiedthree;
+
+  let allschemas;
+
+  beforeEach('Read Schemas from disk', async () => {
+    console.log('Reading Schemas');
+    one = await fs.readJson(path.resolve(__dirname, 'fixtures', 'cyclic', 'one.schema.json'));
+    two = await fs.readJson(path.resolve(__dirname, 'fixtures', 'cyclic', 'two.schema.json'));
+    three = await fs.readJson(path.resolve(__dirname, 'fixtures', 'cyclic', 'three.schema.json'));
+
+    const myloader = loader();
+
+    console.log('Loading Schemas');
+    proxiedone = myloader(one, path.resolve(__dirname, 'fixtures', 'cyclic', 'one.schema.json'));
+    proxiedtwo = myloader(two, path.resolve(__dirname, 'fixtures', 'cyclic', 'two.schema.json'));
+    proxiedthree = myloader(three, path.resolve(__dirname, 'fixtures', 'cyclic', 'three.schema.json'));
+
+    console.log('Traversing Schemas');
+
+    allschemas = traverse([proxiedone, proxiedtwo, proxiedthree]);
+  });
+
+  it('Schemas with cyclic references generate README', () => {
+    const builder = readme({ readme: true });
+    const result = builder([proxiedone, proxiedtwo, proxiedthree]);
+
+    assertMarkdown(result)
+      .contains('http://example.com/schemas/one')
+      .contains('http://example.com/schemas/two')
+      .contains('http://example.com/schemas/three')
+      .contains('http://json-schema.org/draft-04/schema#');
+  });
+
+  it('Schemas with cyclic references generate Markdown', () => {
+    const builder = markdown();
+    const documents = builder(allschemas);
+    assertMarkdown(documents['one-properties-children-items'])
+      .contains('any of');
+
+    assertMarkdown(documents.one)
+      .contains('one-properties-children-items');
+  });
+});

test/fixtures/cyclic/one.schema.json

@@ -0,0 +1,17 @@
+{
+	"$schema": "http://json-schema.org/draft-04/schema#",
+	"$id": "http://example.com/schemas/one",
+
+	"type": "object",
+	"properties": {
+		"children": {
+			"type": "array",
+			"items": {
+				"anyOf": [
+					{ "$ref": "http://example.com/schemas/three" },
+					{ "$ref": "http://example.com/schemas/two" }
+				]
+			}
+		}
+	}
+}

test/fixtures/cyclic/three.schema.json

@@ -0,0 +1,17 @@
+{
+	"$schema": "http://json-schema.org/draft-04/schema#",
+	"$id": "http://example.com/schemas/three",
+
+	"type": "object",
+	"properties": {
+		"children": {
+			"type": "array",
+			"items": {
+				"anyOf": [
+					{ "$ref": "http://example.com/schemas/one" },
+					{ "$ref": "http://example.com/schemas/two" }
+				]
+			}
+		}
+	}
+}

test/fixtures/cyclic/two.schema.json

@@ -0,0 +1,17 @@
+{
+	"$schema": "http://json-schema.org/draft-04/schema#",
+	"$id": "http://example.com/schemas/two",
+
+	"type": "object",
+	"properties": {
+		"children": {
+			"type": "array",
+			"items": {
+				"anyOf": [
+					{ "$ref": "http://example.com/schemas/one" },
+					{ "$ref": "http://example.com/schemas/three" }
+				]
+			}
+		}
+	}
+}

test/traverseSchema.test.js

@@ -12,11 +12,13 @@
 /* eslint-env mocha */
 
 const assert = require('assert');
+const fs = require('fs-extra');
+const path = require('path');
 const {
   loader, filename,
 } = require('../lib/schemaProxy');
 
-const { traverseSchema } = require('../lib/traverseSchema');
+const traverse = require('../lib/traverseSchema');
 
 const example = {
   'meta:license': [
@@ -66,9 +68,27 @@ const example = {
 describe('Testing Schema Traversal', () => {
   it('Schema Traversal generates a list', () => {
     const proxied = loader()(example, 'example.schema.json');
-    const schemas = traverseSchema(proxied);
+    const schemas = traverse([proxied]);
 
     assert.equal(schemas.length, 9);
     assert.equal(schemas[8][filename], 'example.schema.json');
   });
+
+  it('Cyclic Schema Traversal generates a list', async () => {
+    const one = await fs.readJson(path.resolve(__dirname, 'fixtures', 'cyclic', 'one.schema.json'));
+    const two = await fs.readJson(path.resolve(__dirname, 'fixtures', 'cyclic', 'two.schema.json'));
+    const three = await fs.readJson(path.resolve(__dirname, 'fixtures', 'cyclic', 'three.schema.json'));
+
+    const myloader = loader();
+    const proxiedone = myloader(one, path.resolve(__dirname, 'fixtures', 'cyclic', 'one.schema.json'));
+    const proxiedtwo = myloader(two, path.resolve(__dirname, 'fixtures', 'cyclic', 'two.schema.json'));
+    const proxiedthree = myloader(three, path.resolve(__dirname, 'fixtures', 'cyclic', 'three.schema.json'));
+
+
+    const schemas = traverse([proxiedone, proxiedtwo, proxiedthree]);
+
+    assert.equal(schemas[0].$id, 'http://example.com/schemas/one');
+    assert.equal(schemas[4].$id, 'http://example.com/schemas/three');
+    assert.equal(schemas[8].$id, 'http://example.com/schemas/two');
+  });
 });