Skip to content

Commit

Permalink
build(aio): move link disambiguation from getLinkInfo to `getDocFro…
Browse files Browse the repository at this point in the history
…mAlias` (#22494)

The disambiguation needs to be done earlier so that the auto-link-code
post-processor can benefit from it.

PR Close #22494
  • Loading branch information
petebacondarwin authored and alexeagle committed Mar 1, 2018
1 parent 94707fe commit 997b30a
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 85 deletions.
13 changes: 7 additions & 6 deletions aio/tools/transforms/links-package/index.js
Expand Up @@ -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
];
});

This file was deleted.

@@ -0,0 +1,3 @@
module.exports = function disambiguateByDeprecated() {
return (alias, originatingDoc, docs) => docs.filter(doc => doc.deprecated === undefined);
};
@@ -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' },
]);
});
});
@@ -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;
};
};
@@ -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 },
]);
});
});
38 changes: 15 additions & 23 deletions 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);

// 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; });
}
getDocFromAlias.disambiguators = [];
return getDocFromAlias;

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)
);
}
};
35 changes: 17 additions & 18 deletions aio/tools/transforms/links-package/services/getDocFromAlias.spec.js
Expand Up @@ -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']};
Expand All @@ -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'};
aliasMap.addDoc(doc1);
aliasMap.addDoc(doc2);
aliasMap.addDoc(doc3);

expect(getDocFromAlias('a', {area: 'api'})).toEqual([doc1, doc2]);
});
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
];

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'};
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', 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]);
});
});
11 changes: 0 additions & 11 deletions aio/tools/transforms/links-package/services/getLinkInfo.js
Expand Up @@ -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<function(url, title, currentDoc, ambiguousDocs) : array} 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 moduleScopeLinkDisambiguator service for an example).
*/
module.exports = function getLinkInfo(getDocFromAlias, encodeCodeBlock, log) {

getLinkInfoImpl.disambiguators = [];

return getLinkInfoImpl;

function getLinkInfoImpl(url, title, currentDoc) {
Expand All @@ -29,11 +24,6 @@ module.exports = function getLinkInfo(getDocFromAlias, encodeCodeBlock, log) {

var docs = getDocFromAlias(url, currentDoc);

// Give each disambiguator a chance to reduce the number of ambiguous docs
docs = getLinkInfoImpl.disambiguators.reduce(function(docs, disambiguator) {
return disambiguator(url, title, currentDoc, docs);
}, docs);

if (!getLinkInfoImpl.useFirstAmbiguousLink && docs.length > 1) {
linkInfo.valid = false;
linkInfo.errorType = 'ambiguous';
Expand Down Expand Up @@ -80,5 +70,4 @@ module.exports = function getLinkInfo(getDocFromAlias, encodeCodeBlock, log) {

return linkInfo;
}

};

This file was deleted.

0 comments on commit 997b30a

Please sign in to comment.