From 997b30a093a770bd74664233cfff1447ad7dd73f Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 28 Feb 2018 14:52:29 +0000 Subject: [PATCH] build(aio): move link disambiguation from `getLinkInfo` to `getDocFromAlias` (#22494) The disambiguation needs to be done earlier so that the auto-link-code post-processor can benefit from it. PR Close #22494 --- aio/tools/transforms/links-package/index.js | 13 ++++--- .../deprecatedDocsLinkDisambiguator.js | 12 ------ .../disambiguateByDeprecated.js | 3 ++ .../disambiguateByDeprecated.spec.js | 17 +++++++++ .../disambiguators/disambiguateByModule.js | 12 ++++++ .../disambiguateByModule.spec.js | 29 ++++++++++++++ .../links-package/services/getDocFromAlias.js | 38 ++++++++----------- .../services/getDocFromAlias.spec.js | 37 +++++++++--------- .../links-package/services/getLinkInfo.js | 11 ------ .../services/moduleScopeLinkDisambiguator.js | 15 -------- 10 files changed, 101 insertions(+), 86 deletions(-) delete mode 100644 aio/tools/transforms/links-package/services/deprecatedDocsLinkDisambiguator.js create mode 100644 aio/tools/transforms/links-package/services/disambiguators/disambiguateByDeprecated.js create mode 100644 aio/tools/transforms/links-package/services/disambiguators/disambiguateByDeprecated.spec.js create mode 100644 aio/tools/transforms/links-package/services/disambiguators/disambiguateByModule.js create mode 100644 aio/tools/transforms/links-package/services/disambiguators/disambiguateByModule.spec.js delete mode 100644 aio/tools/transforms/links-package/services/moduleScopeLinkDisambiguator.js diff --git a/aio/tools/transforms/links-package/index.js b/aio/tools/transforms/links-package/index.js index 1fab11d2ae..2f6964cfaf 100644 --- a/aio/tools/transforms/links-package/index.js +++ b/aio/tools/transforms/links-package/index.js @@ -8,15 +8,16 @@ module.exports = .factory(require('./services/getAliases')) .factory(require('./services/getDocFromAlias')) .factory(require('./services/getLinkInfo')) - .factory(require('./services/moduleScopeLinkDisambiguator')) - .factory(require('./services/deprecatedDocsLinkDisambiguator')) + .factory(require('./services/disambiguators/disambiguateByDeprecated')) + .factory(require('./services/disambiguators/disambiguateByModule')) .config(function(inlineTagProcessor, linkInlineTagDef) { inlineTagProcessor.inlineTagDefinitions.push(linkInlineTagDef); }) - .config(function( - getLinkInfo, moduleScopeLinkDisambiguator, deprecatedDocsLinkDisambiguator) { - getLinkInfo.disambiguators.push(moduleScopeLinkDisambiguator); - getLinkInfo.disambiguators.push(deprecatedDocsLinkDisambiguator); + .config(function(getDocFromAlias, disambiguateByDeprecated, disambiguateByModule) { + getDocFromAlias.disambiguators = [ + disambiguateByDeprecated, + disambiguateByModule + ]; }); diff --git a/aio/tools/transforms/links-package/services/deprecatedDocsLinkDisambiguator.js b/aio/tools/transforms/links-package/services/deprecatedDocsLinkDisambiguator.js deleted file mode 100644 index 662d3e53e2..0000000000 --- a/aio/tools/transforms/links-package/services/deprecatedDocsLinkDisambiguator.js +++ /dev/null @@ -1,12 +0,0 @@ -var _ = require('lodash'); - -module.exports = function deprecatedDocsLinkDisambiguator() { - return function(url, title, currentDoc, docs) { - if (docs.length != 2) return docs; - - var filteredDocs = _.filter( - docs, function(doc) { return !doc.fileInfo.relativePath.match(/\/(\w+)-deprecated\//); }); - - return filteredDocs.length > 0 ? filteredDocs : docs; - }; -}; diff --git a/aio/tools/transforms/links-package/services/disambiguators/disambiguateByDeprecated.js b/aio/tools/transforms/links-package/services/disambiguators/disambiguateByDeprecated.js new file mode 100644 index 0000000000..44427c9d40 --- /dev/null +++ b/aio/tools/transforms/links-package/services/disambiguators/disambiguateByDeprecated.js @@ -0,0 +1,3 @@ +module.exports = function disambiguateByDeprecated() { + return (alias, originatingDoc, docs) => docs.filter(doc => doc.deprecated === undefined); +}; diff --git a/aio/tools/transforms/links-package/services/disambiguators/disambiguateByDeprecated.spec.js b/aio/tools/transforms/links-package/services/disambiguators/disambiguateByDeprecated.spec.js new file mode 100644 index 0000000000..edc51aa9dd --- /dev/null +++ b/aio/tools/transforms/links-package/services/disambiguators/disambiguateByDeprecated.spec.js @@ -0,0 +1,17 @@ +const disambiguateByDeprecated = require('./disambiguateByDeprecated')(); +const docs = [ + { id: 'doc1' }, + { id: 'doc2', deprecated: true }, + { id: 'doc3', deprecated: '' }, + { id: 'doc4' }, + { id: 'doc5', deprecated: 'Some text' }, +]; + +describe('disambiguateByDeprecated', () => { + it('should filter out docs whose `deprecated` property is defined', () => { + expect(disambiguateByDeprecated('alias', {}, docs)).toEqual([ + { id: 'doc1' }, + { id: 'doc4' }, + ]); + }); +}); \ No newline at end of file diff --git a/aio/tools/transforms/links-package/services/disambiguators/disambiguateByModule.js b/aio/tools/transforms/links-package/services/disambiguators/disambiguateByModule.js new file mode 100644 index 0000000000..8ed131477f --- /dev/null +++ b/aio/tools/transforms/links-package/services/disambiguators/disambiguateByModule.js @@ -0,0 +1,12 @@ +module.exports = function disambiguateByModule() { + return (alias, originatingDoc, docs) => { + const originatingModule = originatingDoc && originatingDoc.moduleDoc; + if (originatingModule) { + const filteredDocs = docs.filter(doc => doc.moduleDoc === originatingModule); + if (filteredDocs.length > 0) { + return filteredDocs; + } + } + return docs; + }; +}; diff --git a/aio/tools/transforms/links-package/services/disambiguators/disambiguateByModule.spec.js b/aio/tools/transforms/links-package/services/disambiguators/disambiguateByModule.spec.js new file mode 100644 index 0000000000..f80e24ae85 --- /dev/null +++ b/aio/tools/transforms/links-package/services/disambiguators/disambiguateByModule.spec.js @@ -0,0 +1,29 @@ +const disambiguateByModule = require('./disambiguateByModule')(); +const moduleA = { name: 'a' }; +const moduleB = { name: 'b' }; +const docs = [ + { id: 'doc1', moduleDoc: moduleA }, + { id: 'doc2', moduleDoc: moduleA }, + { id: 'doc3', moduleDoc: moduleB }, +]; + +describe('disambiguateByModule', () => { + it('should return all docs if the originating doc has no moduleDoc', () => { + expect(disambiguateByModule('alias', { }, docs)).toEqual(docs); + }); + + it('should return all docs if no docs match the originating doc moduleDoc', () => { + expect(disambiguateByModule('alias', { moduleDoc: { name: 'c' } }, docs)).toEqual(docs); + }); + + it('should return only docs that match the moduleDoc of the originating doc', () => { + expect(disambiguateByModule('alias', { moduleDoc: moduleA }, docs)).toEqual([ + { id: 'doc1', moduleDoc: moduleA }, + { id: 'doc2', moduleDoc: moduleA }, + ]); + + expect(disambiguateByModule('alias', { moduleDoc: moduleB }, docs)).toEqual([ + { id: 'doc3', moduleDoc: moduleB }, + ]); + }); +}); \ No newline at end of file diff --git a/aio/tools/transforms/links-package/services/getDocFromAlias.js b/aio/tools/transforms/links-package/services/getDocFromAlias.js index 8860198c3c..3cb8173669 100644 --- a/aio/tools/transforms/links-package/services/getDocFromAlias.js +++ b/aio/tools/transforms/links-package/services/getDocFromAlias.js @@ -1,31 +1,23 @@ -var _ = require('lodash'); - /** * @dgService getDocFromAlias * @description Get an array of docs that match this alias, relative to the originating doc. + * + * @property {Array<(alias: string, originatingDoc: Doc, ambiguousDocs: Doc[]) => Doc[]>} disambiguators + * a collection of functions that attempt to resolve ambiguous links. Each disambiguator returns + * a new collection of docs with unwanted ambiguous docs removed (see links-package/service/disambiguators + * for examples). */ module.exports = function getDocFromAlias(aliasMap) { - return function getDocFromAlias(alias, originatingDoc) { - var docs = aliasMap.getDocs(alias); + getDocFromAlias.disambiguators = []; + return getDocFromAlias; - // If there is more than one item with this name then try to filter them by the originatingDoc's - // area - if (docs.length > 1 && originatingDoc && originatingDoc.area) { - docs = _.filter(docs, function(doc) { return doc.area === originatingDoc.area; }); - } - - // If filtering by area left us with none then let's start again - if (docs.length === 0) { - docs = aliasMap.getDocs(alias); - } - - // If there is more than one item with this name then try to filter them by the originatingDoc's - // module - if (docs.length > 1 && originatingDoc && originatingDoc.module) { - docs = _.filter(docs, function(doc) { return doc.module === originatingDoc.module; }); - } - - return docs; - }; + function getDocFromAlias(alias, originatingDoc) { + return getDocFromAlias.disambiguators.reduce( + // Run the disambiguators while there is more than 1 doc found + (docs, disambiguater) => docs.length > 1 ? disambiguater(alias, originatingDoc, docs) : docs, + // Start with the docs that match the alias + aliasMap.getDocs(alias) + ); + } }; diff --git a/aio/tools/transforms/links-package/services/getDocFromAlias.spec.js b/aio/tools/transforms/links-package/services/getDocFromAlias.spec.js index c5b24a47f2..989ff43ab6 100644 --- a/aio/tools/transforms/links-package/services/getDocFromAlias.spec.js +++ b/aio/tools/transforms/links-package/services/getDocFromAlias.spec.js @@ -3,15 +3,15 @@ var Dgeni = require('dgeni'); var getDocFromAlias, aliasMap; -describe('getDocFromAlias', function() { - beforeEach(function() { +describe('getDocFromAlias', () => { + beforeEach(() => { var dgeni = new Dgeni([testPackage('links-package', true)]); var injector = dgeni.configureInjector(); aliasMap = injector.get('aliasMap'); getDocFromAlias = injector.get('getDocFromAlias'); }); - it('should return an array of docs that match the alias', function() { + it('should return an array of docs that match the alias', () => { var doc1 = {aliases: ['a', 'b', 'c']}; var doc2 = {aliases: ['a', 'b']}; var doc3 = {aliases: ['a']}; @@ -24,25 +24,24 @@ describe('getDocFromAlias', function() { expect(getDocFromAlias('c')).toEqual([doc1]); }); - it('should return docs that match the alias and originating doc\'s area', function() { - var doc1 = {aliases: ['a'], area: 'api'}; - var doc2 = {aliases: ['a'], area: 'api'}; - var doc3 = {aliases: ['a'], area: 'other'}; + it('should filter ambiguous docs by calling each disambiguator', () => { + getDocFromAlias.disambiguators = [ + (alias, originatingDoc, docs) => docs.filter(doc => doc.name.indexOf('X') !== -1), // only if X appears in name + (alias, originatingDoc, docs) => docs.filter(doc => doc.name.indexOf('Y') !== -1) // only if Y appears in name + ]; + + var doc1 = {name: 'X', aliases: ['a', 'b', 'c']}; + var doc2 = {name: 'Y', aliases: ['a', 'b']}; + var doc3 = {name: 'XY', aliases: ['a', 'c']}; aliasMap.addDoc(doc1); aliasMap.addDoc(doc2); aliasMap.addDoc(doc3); - expect(getDocFromAlias('a', {area: 'api'})).toEqual([doc1, doc2]); - }); - - it('should return docs that match the alias and originating doc\'s area and module', function() { - var doc1 = {aliases: ['a'], area: 'api', module: 'ng'}; - var doc2 = {aliases: ['a'], area: 'api', module: 'ngMock'}; - var doc3 = {aliases: ['a'], area: 'other', module: 'ng'}; - aliasMap.addDoc(doc1); - aliasMap.addDoc(doc2); - aliasMap.addDoc(doc3); - - expect(getDocFromAlias('a', {area: 'api', module: 'ng'})).toEqual([doc1]); + // doc1 and doc2 get removed as they don't both have X and Y in name + expect(getDocFromAlias('a')).toEqual([doc3]); + // doc2 gets removed as it has no X; then we have only one doc left so second disambiguator never runs + expect(getDocFromAlias('b')).toEqual([doc1]); + // doc1 gets removed as it has no Y; then we have only one doc left (which would anyway pass 2nd disambiguator) + expect(getDocFromAlias('c')).toEqual([doc3]); }); }); \ No newline at end of file diff --git a/aio/tools/transforms/links-package/services/getLinkInfo.js b/aio/tools/transforms/links-package/services/getLinkInfo.js index fd6b57dab0..7b9eb61237 100644 --- a/aio/tools/transforms/links-package/services/getLinkInfo.js +++ b/aio/tools/transforms/links-package/services/getLinkInfo.js @@ -10,14 +10,9 @@ var path = require('canonical-path'); * @return {Object} The link information * * @property {boolean} relativeLinks Whether we expect the links to be relative to the originating doc - * @property {array 1) { linkInfo.valid = false; linkInfo.errorType = 'ambiguous'; @@ -80,5 +70,4 @@ module.exports = function getLinkInfo(getDocFromAlias, encodeCodeBlock, log) { return linkInfo; } - }; diff --git a/aio/tools/transforms/links-package/services/moduleScopeLinkDisambiguator.js b/aio/tools/transforms/links-package/services/moduleScopeLinkDisambiguator.js deleted file mode 100644 index 08435b96ee..0000000000 --- a/aio/tools/transforms/links-package/services/moduleScopeLinkDisambiguator.js +++ /dev/null @@ -1,15 +0,0 @@ -var _ = require('lodash'); - -module.exports = function moduleScopeLinkDisambiguator() { - return function(url, title, currentDoc, docs) { - if (docs.length > 1) { - // filter out target docs that are not in the same module as the source doc - var filteredDocs = - _.filter(docs, function(doc) { return doc.moduleDoc === currentDoc.moduleDoc; }); - // if all target docs are in a different module then just return the full collection of - // ambiguous docs - return filteredDocs.length > 0 ? filteredDocs : docs; - } - return docs; - }; -};