Skip to content

Commit

Permalink
feat(repository): rejects create/update operations for navigational p…
Browse files Browse the repository at this point in the history
…roperties
  • Loading branch information
Agnes Lin committed Dec 3, 2019
1 parent 77cbd01 commit 95ab583
Show file tree
Hide file tree
Showing 7 changed files with 415 additions and 109 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,142 @@ export function hasManyInclusionResolverAcceptance(
expect(toJSON(result)).to.deepEqual(toJSON(expected));
});

it('throws when navigational properties are present when updating model instance', async () => {
const created = await customerRepo.create({name: 'customer'});
const customerId = created.id;
skipIf(
features.hasRevisionToken,
it,
'returns inclusions after running save() operation',
async () => {
// this shows save() works well with func ensurePersistable and ObjectId
// the test skips for Cloudant as it needs the _rev property for replacement.
// see replace-by-id.suite.ts
const thor = await customerRepo.create({name: 'Thor'});
const odin = await customerRepo.create({name: 'Odin'});

const thorOrder = await orderRepo.create({
customerId: thor.id,
description: 'Pizza',
});

const pizza = await orderRepo.findById(thorOrder.id);
pizza.customerId = odin.id;

await orderRepo.save(pizza);
const odinPizza = await orderRepo.findById(thorOrder.id);

const result = await customerRepo.findById(odin.id, {
include: [{relation: 'orders'}],
});
const expected = {
...odin,
parentId: features.emptyValue,
orders: [
{
...odinPizza,
isShipped: features.emptyValue,
// eslint-disable-next-line @typescript-eslint/camelcase
shipment_id: features.emptyValue,
},
],
};
expect(toJSON(result)).to.containEql(toJSON(expected));
},
);

skipIf(
features.hasRevisionToken,
it,
'returns inclusions after running replaceById() operation',
async () => {
// this shows replaceById() works well with func ensurePersistable and ObjectId
// the test skips for Cloudant as it needs the _rev property for replacement.
// see replace-by-id.suite.ts
const thor = await customerRepo.create({name: 'Thor'});
const odin = await customerRepo.create({name: 'Odin'});

const thorOrder = await orderRepo.create({
customerId: thor.id,
description: 'Pizza',
});

const pizza = await orderRepo.findById(thorOrder.id.toString());
pizza.customerId = odin.id;
// FIXME: [mongo] if pizza obj is converted to JSON obj, it would get an error
// because it tries to convert ObjectId to string type.
// should test with JSON obj once it's fixed.

await orderRepo.replaceById(pizza.id, pizza);
const odinPizza = await orderRepo.findById(thorOrder.id);

const result = await customerRepo.find({
include: [{relation: 'orders'}],
});
const expected = [
{
...thor,
parentId: features.emptyValue,
},
{
...odin,
parentId: features.emptyValue,
orders: [
{
...odinPizza,
isShipped: features.emptyValue,
// eslint-disable-next-line @typescript-eslint/camelcase
shipment_id: features.emptyValue,
},
],
},
];
expect(toJSON(result)).to.deepEqual(toJSON(expected));
},
);

it('returns inclusions after running updateById() operation', async () => {
const thor = await customerRepo.create({name: 'Thor'});
const odin = await customerRepo.create({name: 'Odin'});

const thorOrder = await orderRepo.create({
customerId: thor.id,
description: 'Pizza',
});

const pizza = await orderRepo.findById(thorOrder.id.toString());
pizza.customerId = odin.id;
const reheatedPizza = toJSON(pizza);

await orderRepo.updateById(pizza.id, reheatedPizza);
const odinPizza = await orderRepo.findById(thorOrder.id);

const result = await customerRepo.find({
include: [{relation: 'orders'}],
});
const expected = [
{
...thor,
parentId: features.emptyValue,
},
{
...odin,
parentId: features.emptyValue,
orders: [
{
...odinPizza,
isShipped: features.emptyValue,
// eslint-disable-next-line @typescript-eslint/camelcase
shipment_id: features.emptyValue,
},
],
},
];
expect(toJSON(result)).to.deepEqual(toJSON(expected));
});

it('throws when navigational properties are present when updating model instance with save()', async () => {
// save() calls replaceById so the result will be the same for replaceById
const customer = await customerRepo.create({name: 'customer'});
const anotherCus = await customerRepo.create({name: 'another customer'});
const customerId = customer.id;

await orderRepo.create({
description: 'pizza',
Expand All @@ -201,11 +334,19 @@ export function hasManyInclusionResolverAcceptance(
});
expect(found.orders).to.have.lengthOf(1);

const wrongOrder = await orderRepo.create({
description: 'wrong order',
customerId: anotherCus.id,
});

found.name = 'updated name';
found.orders.push(wrongOrder);

await expect(customerRepo.save(found)).to.be.rejectedWith(
'The `Customer` instance is not valid. Details: `orders` is not defined in the model (value: undefined).',
/Navigational properties are not allowed.*"orders"/,
);
});

// scope for inclusion is not supported yet
it('throws error if the inclusion query contains a non-empty scope', async () => {
const customer = await customerRepo.create({name: 'customer'});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export function hasManyRelationAcceptance(
);
});

it('does not create an array of the related model', async () => {
it('throws when tries to create() an instance with navigational property', async () => {
await expect(
customerRepo.create({
name: 'a customer',
Expand All @@ -184,7 +184,84 @@ export function hasManyRelationAcceptance(
},
],
}),
).to.be.rejectedWith(/`orders` is not defined/);
).to.be.rejectedWith(
'Navigational properties are not allowed in model data (model "Customer" property "orders")',
);
});

it('throws when tries to createAll() instancese with navigational properties', async () => {
await expect(
customerRepo.createAll([
{
name: 'a customer',
orders: [{description: 'order 1'}],
},
{
name: 'a customer',
address: {street: '1 Amedee Bonnet'},
},
]),
).to.be.rejectedWith(
'Navigational properties are not allowed in model data (model "Customer" property "orders")',
);
});

it('throws when the instance contains navigational property when operates update()', async () => {
const created = await customerRepo.create({name: 'customer'});
await orderRepo.create({
description: 'pizza',
customerId: created.id,
});

const found = await customerRepo.findById(created.id, {
include: [{relation: 'orders'}],
});
expect(found.orders).to.have.lengthOf(1);

found.name = 'updated name';
await expect(customerRepo.update(found)).to.be.rejectedWith(
/Navigational properties are not allowed.*"orders"/,
);
});

it('throws when the instancees contain navigational property when operates updateAll()', async () => {
await customerRepo.create({name: 'Mario'});
await customerRepo.create({name: 'Luigi'});

await expect(
customerRepo.updateAll({
name: 'Nintendo',
orders: [{description: 'Switch'}],
}),
).to.be.rejectedWith(/Navigational properties are not allowed.*"orders"/);
});

it('throws when the instance contains navigational property when operates updateById()', async () => {
const customer = await customerRepo.create({name: 'Mario'});

await expect(
customerRepo.updateById(customer.id, {
name: 'Luigi',
orders: [{description: 'Nintendo'}],
}),
).to.be.rejectedWith(/Navigational properties are not allowed.*"orders"/);
});

it('throws when the instance contains navigational property when operates delete()', async () => {
const customer = await customerRepo.create({name: 'customer'});

await orderRepo.create({
description: 'pizza',
customerId: customer.id,
});

const found = await customerRepo.findById(customer.id, {
include: [{relation: 'orders'}],
});

await expect(customerRepo.delete(found)).to.be.rejectedWith(
'Navigational properties are not allowed in model data (model "Customer" property "orders")',
);
});

context('when targeting the source model', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export function hasOneRelationAcceptance(
);

beforeEach(async () => {
await customerRepo.deleteAll();
await addressRepo.deleteAll();
existingCustomerId = (await givenPersistedCustomerInstance()).id;
});
Expand Down
6 changes: 6 additions & 0 deletions packages/repository-tests/src/crud/relations/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import {CrudFeatures, CrudRepositoryCtor} from '../..';
import {
Address,
AddressRepository,
Customer,
CustomerRepository,
Order,
OrderRepository,
Shipment,
ShipmentRepository,
} from './fixtures/models';
import {
Expand All @@ -25,6 +27,10 @@ export function givenBoundCrudRepositories(
repositoryClass: CrudRepositoryCtor,
features: CrudFeatures,
) {
Order.definition.properties.id.type = features.idType;
Address.definition.properties.id.type = features.idType;
Customer.definition.properties.id.type = features.idType;
Shipment.definition.properties.id.type = features.idType;
// when running the test suite on MongoDB, we don't really need to setup
// this config for mongo connector to pass the test.
// however real-world applications might have such config for MongoDB
Expand Down
13 changes: 8 additions & 5 deletions packages/repository-tests/src/crud/replace-by-id.suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,17 @@
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import {Entity, model, property} from '@loopback/repository';
import {AnyObject, EntityCrudRepository} from '@loopback/repository';
import {
AnyObject,
Entity,
EntityCrudRepository,
model,
property,
} from '@loopback/repository';
import {expect, toJSON} from '@loopback/testlab';
import {MixedIdType} from '../helpers.repository-tests';
import {
deleteAllModelsInDefaultDataSource,
MixedIdType,
withCrudCtx,
} from '../helpers.repository-tests';
import {
Expand Down Expand Up @@ -72,7 +77,6 @@ export function createSuiteForReplaceById(
// This important! Not all databases allow `patchById` to set
// properties to "undefined", `replaceById` must always work.
created.description = undefined;

await repo.replaceById(created.id, created);

const found = await repo.findById(created.id);
Expand Down Expand Up @@ -112,7 +116,6 @@ export function createSuiteForReplaceById(
// This important! Not all databases allow `patchById` to set
// properties to "undefined", `replaceById` must always work.
created.description = undefined;

await repo.replaceById(created.id, created);

const found = await repo.findById(created.id);
Expand Down
Loading

0 comments on commit 95ab583

Please sign in to comment.