Skip to content

Commit

Permalink
Merge pull request #1037 from ministryofjustice/fix/deserializer
Browse files Browse the repository at this point in the history
fix: Apply transformer to duplicate resources correctly
  • Loading branch information
teneightfive authored Nov 24, 2020
2 parents 6386a96 + fe1b203 commit c82160f
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 7 deletions.
24 changes: 19 additions & 5 deletions common/lib/api-client/transformers/transform-resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,31 @@ const { cloneDeep } = require('lodash')

module.exports = function transformResource(transformer) {
return function deserializer(item, included) {
if (!transformer) {
return this.deserialize.resource.call(this, item, included)
}

const _this = cloneDeep(this)
const modelName = this.pluralize.singular(item.type)

// Ensure we remove this deserializer to avoid infinite loop
delete _this.models[modelName].options.deserializer

const deserialized = _this.deserialize.resource.call(_this, item, included)
// call devour deserializer
const deserializedData = _this.deserialize.resource.call(
_this,
item,
included
)

if (!transformer) {
return deserialized
}
const transformedData = transformer(deserializedData)

// we need to clear the cache to avoid the original resource being loaded
// the cache is a collection and doesn't allow an item to be updated
this.deserialize.cache.clear()
// set the new transformed data to the cache for future deserializations
this.deserialize.cache.set(item.type, item.id, transformedData)

return transformer(deserialized)
return transformedData
}
}
38 changes: 36 additions & 2 deletions common/lib/api-client/transformers/transform-resource.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ const { cloneDeep } = require('lodash')
const transformResource = require('./transform-resource')

describe('API Client', function () {
describe('Models', function () {
describe('#transform', function () {
describe('Transformers', function () {
describe('#transformResource', function () {
let output, _this, item, included

beforeEach(function () {
Expand All @@ -18,11 +18,16 @@ describe('API Client', function () {
singular: sinon.stub().returnsArg(0),
},
deserialize: {
cache: {
clear: sinon.stub(),
set: sinon.stub().returnsArg(1),
},
resource: sinon.stub().returnsArg(0),
},
}
included = []
item = {
id: '12345',
type: 'book',
foo: 'bar',
}
Expand All @@ -35,10 +40,23 @@ describe('API Client', function () {

it('should not transform output', function () {
expect(output).to.deep.equal({
id: '12345',
type: 'book',
foo: 'bar',
})
})

it('should call original deserializer', function () {
expect(_this.deserialize.resource).to.be.calledOnceWithExactly(
item,
included
)
})

it('should not adjust cache', function () {
expect(_this.deserialize.cache.clear).not.to.be.called
expect(_this.deserialize.cache.set).not.to.be.called
})
})

context('with transformer', function () {
Expand All @@ -61,6 +79,7 @@ describe('API Client', function () {

it('should return transformed resource', function () {
expect(output).to.deep.equal({
id: '12345',
type: 'book',
foo: 'bar',
_fizz: 'buzz',
Expand All @@ -69,6 +88,7 @@ describe('API Client', function () {

it('should not mutate original item', function () {
expect(item).to.deep.equal({
id: '12345',
type: 'book',
foo: 'bar',
})
Expand All @@ -81,6 +101,20 @@ describe('API Client', function () {
)
})

it('should adjust devour cache', function () {
expect(_this.deserialize.cache.clear).to.have.been.calledWithExactly()
expect(_this.deserialize.cache.set).to.have.been.calledWithExactly(
'book',
'12345',
{
id: '12345',
type: 'book',
foo: 'bar',
_fizz: 'buzz',
}
)
})

it.skip('should delete model deserializer', function () {
const _that = cloneDeep(_this)
delete _that.models.book.options.deserializer
Expand Down

0 comments on commit c82160f

Please sign in to comment.