feat(repository): rejects create/update operations for navigational properties

Author: Agnes Lin

packages/repository-tests/src/crud/relations/acceptance/has-many-inclusion-resolver.relation.acceptance.ts

@@ -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',
@@ -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'});

packages/repository-tests/src/crud/relations/acceptance/has-many.relation.acceptance.ts

@@ -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',
@@ -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', () => {

packages/repository-tests/src/crud/relations/acceptance/has-one.relation.acceptance.ts

@@ -49,6 +49,7 @@ export function hasOneRelationAcceptance(
     );
 
     beforeEach(async () => {
+      await customerRepo.deleteAll();
       await addressRepo.deleteAll();
       existingCustomerId = (await givenPersistedCustomerInstance()).id;
     });

packages/repository-tests/src/crud/relations/helpers.ts

@@ -8,9 +8,11 @@ import {CrudFeatures, CrudRepositoryCtor} from '../..';
 import {
   Address,
   AddressRepository,
+  Customer,
   CustomerRepository,
   Order,
   OrderRepository,
+  Shipment,
   ShipmentRepository,
 } from './fixtures/models';
 import {
@@ -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

packages/repository-tests/src/crud/replace-by-id.suite.ts

@@ -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 {
@@ -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);
@@ -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);