fixup! apply review feedback

packages/repository/src/relations/has-one/has-one.decorator.ts

@@ -3,7 +3,6 @@
 // This file is licensed under the MIT License.
 // License text available at https://opensource.org/licenses/MIT
 
-import {property} from '../../decorators/model.decorator';
 import {Entity, EntityResolver} from '../../model';
 import {relation} from '../relation.decorator';
 import {HasOneDefinition, RelationType} from '../relation.types';

packages/repository/src/relations/has-one/has-one.repository.ts

@@ -4,13 +4,12 @@
 // License text available at https://opensource.org/licenses/MIT
 
 import {Getter} from '@loopback/context';
-import {Count, DataObject, Options} from '../../common-types';
+import {DataObject, Options} from '../../common-types';
 import {Entity} from '../../model';
 import {Filter, Where} from '../../query';
 import {
   constrainDataObject,
   constrainFilter,
-  constrainWhere,
 } from '../../repositories/constraint-utils';
 import {EntityCrudRepository} from '../../repositories/repository';
 
@@ -28,21 +27,17 @@ export interface HasOneRepository<Target extends Entity> {
     targetModelData: DataObject<Target>,
     options?: Options,
   ): Promise<Target>;
-  /**
-   * Find target model instance
-   * @param Filter A filter object for where, order, limit, etc.
-   * @param options Options for the operation
-   * @returns A promise which resolves with the found target instance(s)
-   */
-  find(filter?: Filter<Target>, options?: Options): Promise<Target[]>;
 
   /**
    * Find the only target model instance that belongs to the declaring model.
-   * @param filter Query filter
+   * @param filter Query filter without a Where condition
    * @param options Options for the operations
    * @returns A promise of the target object or null if not found.
    */
-  findOne(filter?: Filter<Target>, options?: Options): Promise<Target | null>;
+  get(
+    filter?: Exclude<Filter<Target>, Where<Target>>,
+    options?: Options,
+  ): Promise<Target | undefined>;
 }
 
 export class DefaultHasOneRepository<
@@ -66,32 +61,33 @@ export class DefaultHasOneRepository<
     options?: Options,
   ): Promise<TargetEntity> {
     const targetRepository = await this.getTargetRepository();
-    return targetRepository.create(
-      constrainDataObject(targetModelData, this.constraint),
-      options,
-    );
-  }
-
-  async find(
-    filter?: Filter<TargetEntity>,
-    options?: Options,
-  ): Promise<TargetEntity[]> {
-    const targetRepository = await this.getTargetRepository();
-    return targetRepository.find(
-      constrainFilter(filter, this.constraint),
-      options,
+    // should we have an in memory LUT instead of a db query to increase
+    // performance here?
+    const found = await targetRepository.find(
+      constrainFilter({}, this.constraint),
     );
+    if (found.length > 0) {
+      throw new Error(
+        'HasOne relation does not allow creation of more than one target model instance',
+      );
+    } else {
+      return await targetRepository.create(
+        constrainDataObject(targetModelData, this.constraint),
+        options,
+      );
+    }
   }
 
-  async findOne(
-    filter?: Filter<TargetEntity>,
+  async get(
+    filter?: Exclude<Filter<TargetEntity>, Where<TargetEntity>>,
     options?: Options,
-  ): Promise<TargetEntity | null> {
+  ): Promise<TargetEntity | undefined> {
     const targetRepository = await this.getTargetRepository();
     const found = await targetRepository.find(
       Object.assign({limit: 1}, constrainFilter(filter, this.constraint)),
       options,
     );
+    // TODO: throw EntityNotFound error when target model instance not found
     return found[0];
   }
 }

packages/repository/src/relations/relation.types.ts

@@ -76,7 +76,7 @@ export interface HasOneDefinition extends RelationDefinitionBase {
   /**
    * The foreign key used by the target model.
    *
-   * E.g. when a Customer has many Order instances, then keyTo is "customerId".
+   * E.g. when a Customer has one Address instance, then keyTo is "customerId".
    * Note that "customerId" is the default FK assumed by the framework, users
    * can provide a custom FK name by setting "keyTo".
    */

packages/repository/src/repositories/repository.ts

@@ -185,14 +185,6 @@ export interface EntityCrudRepository<T extends Entity, ID>
    * Promise<false>
    */
   exists(id: ID, options?: Options): Promise<boolean>;
-
-  /**
-   * Find one matching record. Same as 'find', but limited to one result.
-   * @param filter Query filter
-   * @param options Options for the operations
-   * @returns A promise of the target object or null if not found.
-   */
-  findOne(filter?: Filter<T>, options?: Options): Promise<T | null>;
 }
 
 /**
@@ -287,15 +279,6 @@ export class CrudRepositoryImpl<T extends Entity, ID>
     return entities[0];
   }
 
-  findOne(
-    filter?: Filter<T> | undefined,
-    options?: AnyObject | undefined,
-  ): Promise<T | null> {
-    // const found = this.find(Object.assign({limit: 1}, filter), options);
-    // return this.toModel(found);
-    throw new Error('Method not implemented');
-  }
-
   update(entity: DataObject<T>, options?: Options): Promise<void> {
     return this.updateById(this.entityClass.getIdOf(entity), entity, options);
   }

packages/repository/test/acceptance/has-one.relation.acceptance.ts

@@ -11,12 +11,14 @@ import {
   juggler,
   repository,
   RepositoryMixin,
+  Filter,
 } from '../..';
 import {Address} from '../fixtures/models';
 import {CustomerRepository, AddressRepository} from '../fixtures/repositories';
+import {Where} from '../..';
 
-describe.only('hasOne relation', () => {
-  // Given a Customer and Order models - see definitions at the bottom
+describe('hasOne relation', () => {
+  // Given a Customer and Address models - see definitions at the bottom
 
   let app: ApplicationWithRepositories;
   let controller: CustomerController;
@@ -49,6 +51,30 @@ describe.only('hasOne relation', () => {
     expect(persisted.toObject()).to.deepEqual(address.toObject());
   });
 
+  it("doesn't allow to create related model instance twice", async () => {
+    const address = await controller.createCustomerAddress(existingCustomerId, {
+      street: '123 test avenue',
+    });
+    expect(
+      controller.createCustomerAddress(existingCustomerId, {
+        street: '456 test street',
+        zipcode: '44012',
+      }),
+    ).to.be.rejectedWith(
+      /does not allow creation of more than one target model instance/,
+    );
+    expect(address.toObject()).to.containDeep({
+      customerId: existingCustomerId,
+      street: '123 test avenue',
+    });
+
+    const persisted = await addressRepo.findById(address.zipcode);
+    expect(persisted.toObject()).to.deepEqual(address.toObject());
+    expect(addressRepo.findById('44012')).to.be.rejectedWith(
+      /Entity not found/,
+    );
+  });
+
   it('can find instance of the related model', async () => {
     const address = await controller.createCustomerAddress(existingCustomerId, {
       street: '123 test avenue',
@@ -71,6 +97,25 @@ describe.only('hasOne relation', () => {
     expect(persisted[0]).to.deepEqual(foundAddress);
   });
 
+  it('does not allow where filter to find related model instance', async () => {
+    const address = await controller.createCustomerAddress(existingCustomerId, {
+      street: '123 test avenue',
+    });
+
+    const foundAddress = await controller.findCustomerAddressWithFilter(
+      existingCustomerId,
+      {where: {street: '123 test avenue'}},
+    );
+    // TODO: make sure this test fails when where condition is supplied
+    // compiler should have errored out (?)
+    expect(foundAddress).to.containEql(address);
+
+    const persisted = await addressRepo.find({
+      where: {customerId: existingCustomerId},
+    });
+    expect(persisted[0]).to.deepEqual(foundAddress);
+  });
+
   /*---------------- HELPERS -----------------*/
 
   class CustomerController {
@@ -89,7 +134,14 @@ describe.only('hasOne relation', () => {
     }
 
     async findCustomerAddress(customerId: number) {
-      return await this.customerRepository.address(customerId).findOne();
+      return await this.customerRepository.address(customerId).get();
+    }
+
+    async findCustomerAddressWithFilter(
+      customerId: number,
+      filter: Filter<Address>,
+    ) {
+      return await this.customerRepository.address(customerId).get(filter);
     }
   }
 

packages/repository/test/fixtures/models/address.model.ts

@@ -3,7 +3,7 @@
 // This file is licensed under the MIT License.
 // License text available at https://opensource.org/licenses/MIT
 
-import {Entity, model, property, ModelDefinition, belongsTo} from '../../..';
+import {Entity, model, property, belongsTo} from '../../..';
 import {Customer} from './customer.model';
 
 @model()

packages/repository/test/unit/decorator/model-and-relation.decorator.unit.ts

@@ -292,8 +292,11 @@ describe('model decorator', () => {
         RELATIONS_KEY,
         Customer.prototype,
       ) || /* istanbul ignore next */ {};
-    expect(meta.lastOrder).to.eql({
+    expect(meta.lastOrder).to.containEql({
       type: RelationType.hasOne,
+      name: 'lastOrder',
+      target: () => Order,
+      source: Customer,
     });
   });
 

packages/repository/test/unit/repositories/has-one-repository-factory.unit.ts

@@ -15,7 +15,7 @@ import {
   RelationType,
 } from '../../..';
 
-describe.only('createHasOneRepositoryFactory', () => {
+describe('createHasOneRepositoryFactory', () => {
   let customerRepo: CustomerRepository;
 
   beforeEach(givenStubbedCustomerRepo);