Skip to content

Commit

Permalink
feat(links): circular reference link resolution (#217)
Browse files Browse the repository at this point in the history
  • Loading branch information
Khaledgarbaya committed Feb 5, 2018
1 parent 955062c commit a62b179
Show file tree
Hide file tree
Showing 15 changed files with 71 additions and 311 deletions.
9 changes: 6 additions & 3 deletions lib/contentful.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
import axios from '@contentful/axios'
import {createHttpClient, getUserAgentHeader} from 'contentful-sdk-core'
import createContentfulApi from './create-contentful-api'
import createLinkResolver from './create-link-resolver'
import createGlobalOptions from './create-global-options'

/**
* Create a client instance
* @func
Expand Down Expand Up @@ -50,7 +51,9 @@ export function createClient (params) {

// Use resolveLinks param if specified, otherwise default to true
const resolveLinks = !!('resolveLinks' in params ? params.resolveLinks : true)
const shouldLinksResolve = createLinkResolver(resolveLinks)
const removeUnresolved = !!params.removeUnresolved

const getGlobalOptions = createGlobalOptions({resolveLinks, removeUnresolved})
const userAgentHeader = getUserAgentHeader(`contentful.js/${__VERSION__}`,
params.application,
params.integration
Expand All @@ -66,6 +69,6 @@ export function createClient (params) {

return createContentfulApi({
http: http,
shouldLinksResolve: shouldLinksResolve
getGlobalOptions: getGlobalOptions
})
}
16 changes: 8 additions & 8 deletions lib/create-contentful-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ import pagedSync from './paged-sync'
* @param {Object} params - API initialization params
* @prop {Object} http - HTTP client instance
* @prop {Object} entities - Object with wrapper methods for each kind of entity
* @prop {Function} shouldLinksResolve - Link resolver preconfigured with global setting
* @prop {Function} getGlobalOptions - Link resolver preconfigured with global setting
* @return {ClientAPI}
*/
export default function createContentfulApi ({http, shouldLinksResolve}) {
export default function createContentfulApi ({http, getGlobalOptions}) {
const {wrapSpace} = entities.space
const {wrapContentType, wrapContentTypeCollection} = entities.contentType
const {wrapEntry, wrapEntryCollection} = entities.entry
Expand Down Expand Up @@ -179,11 +179,10 @@ export default function createContentfulApi ({http, shouldLinksResolve}) {
* .catch(console.error)
*/
function getEntries (query = {}) {
const resolveLinks = shouldLinksResolve(query)
const resolveForAllLocales = (query.locale && query.locale === '*')
const { resolveLinks, removeUnresolved } = getGlobalOptions(query)
normalizeSelect(query)
return http.get('entries', createRequestConfig({query: query}))
.then((response) => wrapEntryCollection(response.data, resolveLinks, resolveForAllLocales), errorHandler)
.then((response) => wrapEntryCollection(response.data, { resolveLinks, removeUnresolved }), errorHandler)
}
/**
* Gets an Asset
Expand Down Expand Up @@ -265,8 +264,8 @@ export default function createContentfulApi ({http, shouldLinksResolve}) {
* .catch(console.error)
*/
function sync (query = {}) {
const resolveLinks = shouldLinksResolve(query)
return pagedSync(http, query, resolveLinks)
const { resolveLinks, removeUnresolved } = getGlobalOptions(query)
return pagedSync(http, query, { resolveLinks, removeUnresolved })
}

/**
Expand Down Expand Up @@ -300,7 +299,8 @@ export default function createContentfulApi ({http, shouldLinksResolve}) {
* console.log( parsedData.items[0].fields.foo ); // foo
*/
function parseEntries (data) {
return wrapEntryCollection(data, true, false)
const { resolveLinks, removeUnresolved } = getGlobalOptions({})
return wrapEntryCollection(data, { resolveLinks, removeUnresolved })
}
/*
* sdk relies heavily on sys metadata
Expand Down
6 changes: 3 additions & 3 deletions lib/create-link-resolver.js → lib/create-global-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
* @param {boolean} globalSetting - Global library setting for link resolution
* @returns {function} Link resolver method preconfigured with global setting
*/
export default function createLinkResolver (globalSetting) {
export default function createGlobalOptions (globalSettings) {
/**
* Link resolver method
* @param {Object} query - regular query object used for collection endpoints
*/
return function shouldLinksResolve (query) {
return !!('resolveLinks' in query ? query.resolveLinks : globalSetting)
return function getGlobalOptions (query) {
return Object.assign({}, globalSettings, query)
}
}
14 changes: 4 additions & 10 deletions lib/entities/entry.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import cloneDeep from 'lodash/cloneDeep'
import {toPlainObject, freezeSys} from 'contentful-sdk-core'
import mixinLinkGetters from '../mixins/link-getters'
import mixinStringifySafe from '../mixins/stringify-safe'
import resolveResponse from 'contentful-resolve-response'

/**
* Types of fields found in an Entry
Expand Down Expand Up @@ -94,19 +94,13 @@ export function wrapEntry (data) {
* Data is also mixed in with link getters if links exist and includes were requested
* @private
* @param {Object} data - Raw entry collection data
* @param {Object} options - wrapper options
* @return {EntryCollection} Wrapped entry collection data
*/
export function wrapEntryCollection (data, resolveLinks, resolveForAllLocales) {
export function wrapEntryCollection (data, {resolveLinks, removeUnresolved}) {
const wrappedData = mixinStringifySafe(toPlainObject(cloneDeep(data)))
if (resolveLinks) {
const includes = prepareIncludes(wrappedData.includes, wrappedData.items)
mixinLinkGetters(wrappedData.items, includes, resolveForAllLocales)
wrappedData.items = resolveResponse(wrappedData, {removeUnresolved})
}
return freezeSys(wrappedData)
}

function prepareIncludes (includes = {}, items) {
includes.Entry = includes.Entry || []
includes.Entry = [ ...new Set([...includes.Entry, ...items]) ]
return includes
}
104 changes: 0 additions & 104 deletions lib/mixins/link-getters.js

This file was deleted.

21 changes: 3 additions & 18 deletions lib/paged-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@
* See <a href="https://www.contentful.com/developers/docs/concepts/sync/">Synchronization</a> for more information.
* @namespace Sync
*/
import cloneDeep from 'lodash/cloneDeep'
import {createRequestConfig, freezeSys, toPlainObject} from 'contentful-sdk-core'
import mixinLinkGetters from './mixins/link-getters'
import resolveResponse from 'contentful-resolve-response'
import mixinStringifySafe from './mixins/stringify-safe'

/**
Expand Down Expand Up @@ -41,7 +40,7 @@ import mixinStringifySafe from './mixins/stringify-safe'
* @param {boolean} resolveLinks - If links should be resolved
* @return {Promise<SyncCollection>}
*/
export default function pagedSync (http, query, resolveLinks) {
export default function pagedSync (http, query, { resolveLinks, removeUnresolved }) {
if (!query || (!query.initial && !query.nextSyncToken)) {
throw new Error('Please provide one of `initial` or `nextSyncToken` parameters for syncing')
}
Expand All @@ -56,7 +55,7 @@ export default function pagedSync (http, query, resolveLinks) {
.then((response) => {
// clones response.items used in includes because we don't want these to be mutated
if (resolveLinks) {
mixinLinkGetters(response.items, mapIncludeItems(cloneDeep(response.items)))
response.items = resolveResponse(response, {resolveLinks, removeUnresolved})
}
// maps response items again after getters are attached
const mappedResponseItems = mapResponseItems(response.items)
Expand All @@ -81,20 +80,6 @@ function mapResponseItems (items) {
}
}

/**
* Creates an object similar to the one retrieved on `includes` from the `entries`
* endpoint, for usage with the link getters mixin
* @private
* @param {Array<Entities.Entry|Entities.Array|Sync.DeletedEntry|Sync.DeletedAsset>} items
* @return {Object}
*/
function mapIncludeItems (items) {
return {
Entry: items.filter((item) => item.sys.type === 'Entry'),
Asset: items.filter((item) => item.sys.type === 'Asset')
}
}

/**
* If the response contains a nextPageUrl, extracts the sync token to get the
* next page and calls itself again with that token.
Expand Down
11 changes: 6 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@
"pretest": "rimraf coverage && npm run lint",
"test": "npm run test:cover && npm run test:integration && npm run test:browser-local && npm run test:size",
"test:ci": "npm run build && ./node_modules/contentful-sdk-core/bin/test-ci.sh",
"test:cover": "BABEL_ENV=test babel-node ./node_modules/istanbul/lib/cli.js cover ./test/runner | tap-spec",
"test:unit": "BABEL_ENV=test babel-node ./test/runner | tap-spec",
"test:debug": "BABEL_ENV=test babel-node debug ./test/runner",
"test:integration": "BABEL_ENV=test babel-node ./test/integration/tests.js | tap-spec",
"test:integration-debug": "BABEL_ENV=test babel-node debug ./test/integration/tests.js",
"test:cover": "BABEL_ENV=test node ./node_modules/istanbul/lib/cli.js cover ./test/runner | tap-spec",
"test:unit": "BABEL_ENV=test node ./test/runner | tap-spec",
"test:debug": "BABEL_ENV=test node debug ./test/runner",
"test:integration": "BABEL_ENV=test babel-node ./test/integration/runner.js | tap-spec",
"test:integration-debug": "BABEL_ENV=test babel-node debug ./test/integration/runner.js",
"test:browser-local": "BABEL_ENV=test karma start karma.conf.local.js",
"test:browser-remote": "BABEL_ENV=test karma start karma.conf.saucelabs.js",
"test:e2e": "node test/e2e/index.js",
Expand All @@ -58,6 +58,7 @@
],
"dependencies": {
"@contentful/axios": "^0.18.0",
"contentful-resolve-response": "^1.0.0",
"contentful-sdk-core": "^5.0.1",
"json-stringify-safe": "^5.0.1",
"lodash": "^4.17.4"
Expand Down
5 changes: 5 additions & 0 deletions test/integration/runner.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
require('babel-core/register')({
ignore: /node_modules(?!\/contentful-resolve-response)/
})

require('./tests.js')
5 changes: 4 additions & 1 deletion test/runner
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#!/usr/bin/env node
require('babel-core/register')({
ignore: /node_modules(?!\/contentful-resolve-response)/
})
require('require-all')({
dirname: process.cwd() + '/test/unit',
filter: process.argv[2] || /\-test\.js$/,
filter: process.argv[2] || /-test\.js$/,
recursive: true
})
8 changes: 4 additions & 4 deletions test/unit/contentful-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ test('Initializes API with link resolution turned on by default', (t) => {
const apiStub = sinon.stub().returns({})
createClientRewireApi.__Rewire__('createContentfulApi', apiStub)
createClient({accessToken: 'accesstoken', space: 'spaceid'})
t.ok(apiStub.args[0][0].shouldLinksResolve({}), 'not overriden by query')
t.notOk(apiStub.args[0][0].shouldLinksResolve({resolveLinks: false}), 'overriden by query')
t.ok(apiStub.args[0][0].getGlobalOptions({}).resolveLinks, 'not overriden by query')
t.notOk(apiStub.args[0][0].getGlobalOptions({resolveLinks: false}).resolveLinks, 'overriden by query')
createClientRewireApi.__ResetDependency__('createHttpClient')
createClientRewireApi.__ResetDependency__('rateLimit')
t.end()
Expand All @@ -94,8 +94,8 @@ test('Initializes API with link resolution turned on explicitly', (t) => {
const apiStub = sinon.stub().returns({})
createClientRewireApi.__Rewire__('createContentfulApi', apiStub)
createClient({accessToken: 'accesstoken', space: 'spaceid', resolveLinks: true})
t.ok(apiStub.args[0][0].shouldLinksResolve({}), 'not overriden by query')
t.notOk(apiStub.args[0][0].shouldLinksResolve({resolveLinks: false}), 'overriden by query')
t.ok(apiStub.args[0][0].getGlobalOptions({}).resolveLinks, 'not overriden by query')
t.notOk(apiStub.args[0][0].getGlobalOptions({resolveLinks: false}).resolveLinks, 'overriden by query')
createClientRewireApi.__ResetDependency__('createHttpClient')
createClientRewireApi.__ResetDependency__('rateLimit')
t.end()
Expand Down
Loading

0 comments on commit a62b179

Please sign in to comment.