fix(repository): revert hasOne target FK as PK implementation

examples/todo-list/src/models/todo-list-image.model.ts

@@ -1,9 +1,15 @@
-import {Entity, model, property, belongsToUniquely} from '@loopback/repository';
+import {Entity, model, property, belongsTo} from '@loopback/repository';
 import {TodoList} from './todo-list.model';
 
 @model()
 export class TodoListImage extends Entity {
-  @belongsToUniquely(() => TodoList)
+  @property({
+    type: 'number',
+    id: true,
+  })
+  id: number;
+
+  @belongsTo(() => TodoList)
   todoListId?: number;
 
   @property({

examples/todo-list/src/repositories/index.ts

@@ -5,3 +5,4 @@
 
 export * from './todo.repository';
 export * from './todo-list.repository';
+export * from './todo-list-image.repository';

examples/todo-list/src/repositories/todo-list-image.repository.ts

@@ -10,11 +10,11 @@ import {TodoListRepository} from './todo-list.repository';
 
 export class TodoListImageRepository extends DefaultCrudRepository<
   TodoListImage,
-  typeof TodoListImage.prototype.todoListId
+  typeof TodoListImage.prototype.id
 > {
   public readonly todoList: BelongsToAccessor<
     TodoList,
-    typeof TodoListImage.prototype.todoListId
+    typeof TodoListImage.prototype.id
   >;
   constructor(
     @inject('datasources.db') dataSource: DbDataSource,

examples/todo-list/test/acceptance/todo-list-image.acceptance.ts

@@ -0,0 +1,130 @@
+// Copyright IBM Corp. 2018. All Rights Reserved.
+// Node module: @loopback/example-todo-list
+// This file is licensed under the MIT License.
+// License text available at https://opensource.org/licenses/MIT
+
+import {
+  Client,
+  createRestAppClient,
+  expect,
+  givenHttpServerConfig,
+  toJSON,
+} from '@loopback/testlab';
+import {TodoListApplication} from '../../src/application';
+import {TodoList, TodoListImage} from '../../src/models/';
+import {
+  TodoListRepository,
+  TodoListImageRepository,
+} from '../../src/repositories/';
+import {givenTodoListImage, givenTodoList} from '../helpers';
+
+describe('TodoListApplication', () => {
+  let app: TodoListApplication;
+  let client: Client;
+  let todoListImageRepo: TodoListImageRepository;
+  let todoListRepo: TodoListRepository;
+
+  let persistedTodoList: TodoList;
+
+  before(givenRunningApplicationWithCustomConfiguration);
+  after(() => app.stop());
+
+  before(givenTodoListImageRepository);
+  before(givenTodoListRepository);
+  before(() => {
+    client = createRestAppClient(app);
+  });
+
+  beforeEach(async () => {
+    await todoListImageRepo.deleteAll();
+    await todoListRepo.deleteAll();
+  });
+
+  beforeEach(async () => {
+    persistedTodoList = await givenTodoListInstance();
+  });
+
+  it('creates image for a todoList', async () => {
+    const todoListImage = givenTodoListImage();
+    delete todoListImage.todoListId;
+    const response = await client
+      .post(`/todo-lists/${persistedTodoList.id}/image`)
+      .send(todoListImage)
+      .expect(200);
+
+    const expected = {...todoListImage, todoListId: persistedTodoList.id};
+    expect(response.body).to.containEql(expected);
+
+    const created = await todoListImageRepo.findById(response.body.id);
+    expect(toJSON(created)).to.deepEqual({id: response.body.id, ...expected});
+  });
+
+  it('finds images for a todoList', async () => {
+    const todoListImage = await givenTodoListImageInstanceOfTodoList(
+      persistedTodoList.id,
+      {
+        value: 'A picture of a green checkmark',
+      },
+    );
+
+    const response = await client
+      .get(`/todo-lists/${persistedTodoList.id}/image`)
+      .send()
+      .expect(200);
+
+    expect(response.body).to.containDeep(todoListImage);
+  });
+
+  /*
+   ============================================================================
+   TEST HELPERS
+   These functions help simplify setup of your test fixtures so that your tests
+   can:
+   - operate on a "clean" environment each time (a fresh in-memory database)
+   - avoid polluting the test with large quantities of setup logic to keep
+   them clear and easy to read
+   - keep them DRY (who wants to write the same stuff over and over?)
+   ============================================================================
+   */
+
+  async function givenRunningApplicationWithCustomConfiguration() {
+    app = new TodoListApplication({
+      rest: givenHttpServerConfig(),
+    });
+
+    await app.boot();
+
+    /**
+     * Override default config for DataSource for testing so we don't write
+     * test data to file when using the memory connector.
+     */
+    app.bind('datasources.config.db').to({
+      name: 'db',
+      connector: 'memory',
+    });
+
+    // Start Application
+    await app.start();
+  }
+
+  async function givenTodoListImageRepository() {
+    todoListImageRepo = await app.getRepository(TodoListImageRepository);
+  }
+
+  async function givenTodoListRepository() {
+    todoListRepo = await app.getRepository(TodoListRepository);
+  }
+
+  async function givenTodoListImageInstanceOfTodoList(
+    id: typeof TodoList.prototype.id,
+    todoListImage?: Partial<TodoListImage>,
+  ) {
+    const data = givenTodoListImage(todoListImage);
+    delete data.todoListId;
+    return await todoListRepo.image(id).create(data);
+  }
+
+  async function givenTodoListInstance(todoList?: Partial<TodoList>) {
+    return await todoListRepo.create(givenTodoList(todoList));
+  }
+});

examples/todo-list/test/helpers.ts

@@ -3,7 +3,7 @@
 // This file is licensed under the MIT License.
 // License text available at https://opensource.org/licenses/MIT
 
-import {Todo, TodoList} from '../src/models';
+import {Todo, TodoList, TodoListImage} from '../src/models';
 
 /*
  ==============================================================================
@@ -55,3 +55,17 @@ export function givenTodoList(todoList?: Partial<TodoList>) {
   );
   return new TodoList(data);
 }
+
+/**
+ * Generate a complete TodoListImage object for use with tests.
+ * @param todoListImage A partial (or complete) TodoListImage object.
+ */
+export function givenTodoListImage(todoListImage?: Partial<TodoListImage>) {
+  const data = Object.assign(
+    {
+      value: 'A picture of a basket of fruits',
+    },
+    todoListImage,
+  );
+  return new TodoListImage(data);
+}

packages/repository/src/relations/belongs-to/belongs-to.decorator.ts

@@ -14,14 +14,11 @@ import {BelongsToDefinition, RelationType} from '../relation.types';
  * @param targetResolver A resolver function that returns the target model for
  * a belongsTo relation
  * @param definition Optional metadata for setting up a belongsTo relation
- * @param propertyMeta Optional property metadata to call the proprety decorator
- * with
  * @returns {(target: Object, key:string)}
  */
 export function belongsTo<T extends Entity>(
   targetResolver: EntityResolver<T>,
   definition?: Partial<BelongsToDefinition>,
-  propertyMeta?: Partial<PropertyDefinition>,
 ) {
   return function(decoratedTarget: Entity, decoratedKey: string) {
     const propMeta: PropertyDefinition = {
@@ -33,7 +30,6 @@ export function belongsTo<T extends Entity>(
       // allows controller methods to exclude required properties
       // required: true,
     };
-    Object.assign(propMeta, propertyMeta);
     property(propMeta)(decoratedTarget, decoratedKey);
 
     // @belongsTo() is typically decorating the foreign key property,
@@ -58,22 +54,3 @@ export function belongsTo<T extends Entity>(
     relation(meta)(decoratedTarget, decoratedKey);
   };
 }
-
-/**
- * Sugar syntax for belongsTo decorator for hasOne relation. Calls belongsTo
- * with property definition defaults for non-generated id field
- * @param targetResolver A resolver function that returns the target model for
- * a belongsTo relation
- * @param definition Optional metadata for setting up a belongsTo relation
- * @returns {(target: Object, key:string)}
- */
-export function belongsToUniquely<T extends Entity>(
-  targetResolver: EntityResolver<T>,
-  definition?: Partial<BelongsToDefinition>,
-) {
-  const propertyMetaDefaults: Partial<PropertyDefinition> = {
-    id: true,
-    generated: false,
-  };
-  return belongsTo(targetResolver, definition, propertyMetaDefaults);
-}

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

@@ -99,11 +99,5 @@ function resolveHasOneMetadata(
     throw new InvalidRelationError(reason, relationMeta);
   }
 
-  const defaultFkProp = targetModel.definition.properties[defaultFkName];
-  if (!defaultFkProp.id || defaultFkProp.generated) {
-    const reason = `foreign key ${defaultFkName} must be an id property that is not auto generated`;
-    throw new InvalidRelationError(reason, relationMeta);
-  }
-
   return Object.assign(relationMeta, {keyTo: defaultFkName});
 }

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

@@ -47,7 +47,7 @@ export class DefaultHasOneRepository<
   TargetRepository extends EntityCrudRepository<TargetEntity, TargetID>
 > implements HasOneRepository<TargetEntity> {
   /**
-   * Constructor of DefaultHasManyEntityCrudRepository
+   * Constructor of DefaultHasOneEntityCrudRepository
    * @param getTargetRepository the getter of the related target model repository instance
    * @param constraint the key value pair representing foreign key name to constrain
    * the target repository instance

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

@@ -44,11 +44,14 @@ describe('hasOne relation', () => {
       street: '123 test avenue',
     });
 
-    const persisted = await addressRepo.findById(address.customerId);
+    const persisted = await addressRepo.findById(address.zipcode);
     expect(persisted.toObject()).to.deepEqual(address.toObject());
   });
 
-  it('refuses to create related model instance twice', async () => {
+  // We do not enforce referential integrity at the moment. It is up to
+  // our users to set up unique constraint(s) between related models at the
+  // database level
+  it.skip('refuses to create related model instance twice', async () => {
     const address = await controller.createCustomerAddress(existingCustomerId, {
       street: '123 test avenue',
     });
@@ -62,7 +65,7 @@ describe('hasOne relation', () => {
       street: '123 test avenue',
     });
 
-    const persisted = await addressRepo.findById(address.customerId);
+    const persisted = await addressRepo.findById(address.zipcode);
     expect(persisted.toObject()).to.deepEqual(address.toObject());
   });
 
@@ -106,7 +109,7 @@ describe('hasOne relation', () => {
     const address = await controller.createCustomerAddress(existingCustomerId, {
       street: '123 test avenue',
     });
-    await addressRepo.deleteById(address.customerId);
+    await addressRepo.deleteById(address.zipcode);
 
     await expect(
       controller.findCustomerAddress(existingCustomerId),

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, belongsToUniquely} from '../../..';
+import {Entity, model, property, belongsTo} from '../../..';
 import {Customer} from './customer.model';
 
 @model()
@@ -14,6 +14,7 @@ export class Address extends Entity {
   street: String;
   @property({
     type: 'string',
+    id: true,
   })
   zipcode: String;
   @property({
@@ -25,6 +26,6 @@ export class Address extends Entity {
   })
   province: String;
 
-  @belongsToUniquely(() => Customer)
+  @belongsTo(() => Customer)
   customerId: number;
 }

packages/repository/test/fixtures/repositories/address.repository.ts

@@ -15,11 +15,11 @@ import {CustomerRepository} from '../repositories';
 
 export class AddressRepository extends DefaultCrudRepository<
   Address,
-  typeof Address.prototype.customerId
+  typeof Address.prototype.zipcode
 > {
   public readonly customer: BelongsToAccessor<
     Customer,
-    typeof Address.prototype.customerId
+    typeof Address.prototype.zipcode
   >;
 
   constructor(