Skip to content

Commit

Permalink
fix: 🐛 fix retrieved fields from the database when a smart field is u…
Browse files Browse the repository at this point in the history
…sed as a reference field (#505)
  • Loading branch information
ghusse committed Sep 8, 2020
1 parent a7e50f9 commit 14286fb
Show file tree
Hide file tree
Showing 5 changed files with 231 additions and 25 deletions.
2 changes: 1 addition & 1 deletion src/services/has-many-getter.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function HasManyGetter(model, association, opts, params) {
const primaryKeyModel = _.keys(model.primaryKeys)[0];

function getFieldNamesRequested() {
return extractRequestedFields(params.fields, association);
return extractRequestedFields(params.fields, association, Interface.Schemas.schemas);
}

const fieldNamesRequested = getFieldNamesRequested();
Expand Down
57 changes: 45 additions & 12 deletions src/services/requested-fields-extractor.js
Original file line number Diff line number Diff line change
@@ -1,28 +1,62 @@
/**
* @typedef {{
* fields: Array<{
* field: string;
* isVirtual: boolean;
* }>
* }} Schema
*/

/**
* @param {string} associationName
* @param {string[]} requestedFields
* @param {Schema} schema
* @returns {string[]}
*/
function extractRequestedFieldsForAssociation(associationName, requestedFields, schema) {
const referencesSmartFields = requestedFields.some(
(fieldName) => schema.fields.some((field) => field.field === fieldName && field.isVirtual),
);

// TODO: have a way of specifying which actual field is used for the computation of a smart field
// and use that info to only retrieve fields that are needed
if (referencesSmartFields) {
return [`${associationName}`];
}

return requestedFields.map((fieldName) => `${associationName}.${fieldName}`);
}

/**
* @param {Record<string, string>} fields
* @param {Record<string, string>} requestedFields
* @param {*} modelOrAssociation
* @param {{
* [collection: string]: Schema
* }} schemas
* @returns {string[]}
*/
function extractRequestedFields(fields, modelOrAssociation) {
if (!fields || !fields[modelOrAssociation.name]) { return null; }
function extractRequestedFields(requestedFields, modelOrAssociation, schemas) {
if (!requestedFields || !requestedFields[modelOrAssociation.name]) { return null; }

// NOTICE: Force the primaryKey retrieval to store the records properly in
// the client.
const primaryKeyArray = [Object.keys(modelOrAssociation.primaryKeys)[0]];

const allAssociationFields = Object.keys(modelOrAssociation.associations)
// NOTICE: Remove fields for which attributes are not explicitely set
// NOTICE: Remove fields for which attributes are not explicitly set
// in the requested fields
.filter((associationName) => fields[associationName])
.map((associationName) => {
const associationFields = fields[associationName].split(',');
return associationFields.map((fieldName) => `${associationName}.${fieldName}`);
})
.filter((associationName) => requestedFields[associationName])
.map((associationName) => extractRequestedFieldsForAssociation(
associationName,
requestedFields && requestedFields[associationName]
? requestedFields[associationName].split(',')
: [],
schemas[modelOrAssociation.associations[associationName].target.name],
))
.flat();

const modelFields = fields[modelOrAssociation.name].split(',')
.filter((fieldName) => !fields[fieldName]);
const modelFields = requestedFields[modelOrAssociation.name].split(',')
.filter((fieldName) => !requestedFields[fieldName]);

return Array.from(new Set([
...primaryKeyArray,
Expand All @@ -31,5 +65,4 @@ function extractRequestedFields(fields, modelOrAssociation) {
]));
}


module.exports = extractRequestedFields;
2 changes: 1 addition & 1 deletion src/services/resources-getter.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function ResourcesGetter(model, options, params) {
associations.push(associationFromSorting);
}

const requestedFields = extractRequestedFields(params.fields, model);
const requestedFields = extractRequestedFields(params.fields, model, Schemas.schemas);

return _.union(
associations,
Expand Down
105 changes: 105 additions & 0 deletions test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ const HasManyDissociator = require('../src/services/has-many-dissociator');
{ field: 'resetPasswordToken', type: 'String' },
{ field: 'addresses', type: ['Number'] },
{ field: 'uuid', type: 'String' },
{ field: 'fullName', isVirtual: true, type: 'String' },
],
},
bike: {
Expand Down Expand Up @@ -1367,6 +1368,54 @@ const HasManyDissociator = require('../src/services/has-many-dissociator');
expect(count).toStrictEqual(2);
});
});

describe('request on the resources getter with a smart field', () => {
it('should only retrieve requested fields when only DB fields are used', async () => {
expect.assertions(5);
const params = {
fields: {
address: 'user',
user: 'firstName',
},
page: { number: '1' },
timezone: 'Europe/Paris',
};
const result = await new ResourcesGetter(
models.address,
sequelizeOptions,
params,
).perform();

expect(result[0]).not.toHaveLength(0);
expect(result[0][0]).toHaveProperty('user');
expect(result[0][0].user.dataValues).toHaveProperty('firstName');
expect(result[0][0].user.dataValues).toHaveProperty('id');
expect(result[0][0].user.dataValues).not.toHaveProperty('lastName');
});

it('should retrieve all fields when a smart field is requested', async () => {
expect.assertions(5);
const params = {
fields: {
address: 'user',
user: 'fullName',
},
page: { number: '1' },
timezone: 'Europe/Paris',
};
const result = await new ResourcesGetter(
models.address,
sequelizeOptions,
params,
).perform();

expect(result[0]).not.toHaveLength(0);
expect(result[0][0]).toHaveProperty('user');
expect(result[0][0].user.dataValues).toHaveProperty('firstName');
expect(result[0][0].user.dataValues).toHaveProperty('id');
expect(result[0][0].user.dataValues).toHaveProperty('lastName');
});
});
});

describe('hasmany > has-many-getter', () => {
Expand Down Expand Up @@ -1516,6 +1565,62 @@ const HasManyDissociator = require('../src/services/has-many-dissociator');
expect(count).toStrictEqual(4);
});
});

describe('request on the has-many-getter with a smart field for an association', () => {
it('should get all fields for addresses when a smart field is requested', async () => {
expect.assertions(4);
const params = {
recordId: 100,
associationName: 'addresses',
fields: {
address: 'street',
user: 'fullName',
},
page: { number: '1', size: '20' },
timezone: 'Europe/Paris',
};
const result = await new HasManyGetter(
models.user,
models.address,
sequelizeOptions,
params,
)
.perform();

expect(result[0]).not.toHaveLength(0);
expect(result[0][0].user.dataValues).toHaveProperty('id');
expect(result[0][0].user.dataValues).toHaveProperty('firstName');
expect(result[0][0].user.dataValues).toHaveProperty('lastName');
});

it('should get only requested fields on the related users', async () => {
expect.assertions(5);
const params = {
recordId: 100,
associationName: 'addresses',
fields: {
address: 'street',
user: 'firstName',
},
page: { number: '1', size: '20' },
timezone: 'Europe/Paris',
};

const result = await new HasManyGetter(
models.user,
models.address,
sequelizeOptions,
params,
)
.perform();

expect(result[0]).not.toHaveLength(0);
expect(result[0][0]).toHaveProperty('user');
expect(result[0][0].user.dataValues).toHaveProperty('firstName');
expect(result[0][0].user.dataValues).toHaveProperty('id');
expect(result[0][0].user.dataValues).not.toHaveProperty('lastName');
});
});
});

describe('request on the has-many-getter with a search parameter', () => {
Expand Down
90 changes: 79 additions & 11 deletions test/services/requested-fields-extractor.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,49 +53,117 @@ describe('services > requested-fields-extractor', () => {

const fields = {
user: 'name',
address: 'street',
homeAddress: 'street',
};

const model = {
name: 'user',
primaryKeys: { id: null, uid: null },
associations: {
address: {
name: 'address',
homeAddress: {
name: 'homeAddress',
target: {
name: 'addresses',
},
},
},
};

const result = extractRequestedFields(fields, model);
const schemas = {
addresses: {
fields: [{
field: 'street',
isVirtual: false,
}],
},
};

const result = extractRequestedFields(fields, model, schemas);

expect(result).toStrictEqual(['id', 'name', 'address.street']);
expect(result).toStrictEqual(['id', 'name', 'homeAddress.street']);
});

it('should remove associations from fields if there are explicit fields requested', () => {
expect.assertions(1);

const fields = {
user: 'name,address,account',
address: 'street',
user: 'name,homeAddress,account',
homeAddress: 'street',
};

const model = {
name: 'user',
primaryKeys: { id: null, uid: null },
associations: {
address: {
name: 'address',
homeAddress: {
name: 'homeAddress',
target: {
name: 'addresses',
},
},
},
};

const result = extractRequestedFields(fields, model);
const schemas = {
addresses: {
fields: [
{
field: 'street',
isVirtual: false,
},
],
},
};

const result = extractRequestedFields(fields, model, schemas);

expect(result).toStrictEqual([
'id',
'name',
'account',
'homeAddress.street',
]);
});

it('should include all fields from an association for which a smart field is requested', () => {
expect.assertions(1);

const fields = {
user: 'name,homeAddress,account',
homeAddress: 'street',
};

const model = {
name: 'user',
primaryKeys: { id: null, uid: null },
associations: {
homeAddress: {
name: 'homeAddress',
target: {
name: 'addresses',
},
},
},
};

const schemas = {
addresses: {
fields: [
{
field: 'street',
isVirtual: true,
},
],
},
};

const result = extractRequestedFields(fields, model, schemas);

expect(result).toStrictEqual([
'id',
'name',
'account',
'address.street',
'homeAddress',
]);
});
});

0 comments on commit 14286fb

Please sign in to comment.