feat(repository): initial AtomicCrudRepository implementation

[b-admike]

Connect to #1956. Implement an atomic findOrCreate function with a new interface for atomic operations as suggested in #1956 with unit tests (most copied from DefaultCrudRepository). Needs loopbackio/loopback-datasource-juggler#1654 to land first.

Checklist

• npm test passes on your machine
• New tests added or existing tests modified to cover all changes
• Code conforms with the style guide
• API Documentation in code was updated
• Documentation in /docs/site was updated
• Affected artifact templates in packages/cli were updated
• Affected example projects in examples/* were updated

[bajtos]

Please add an end-to-end test to verify that when a connector does not support atomic findOrCreate, the HTTP responses have status code set to 501 Not Implemented. A pointer to help you find the place where to implement this functionality:
https://github.com/strongloop/loopback-next/blob/1bcdb5b69221501b3952bd70c0c78409b600d1ab/packages/rest/src/providers/reject.provider.ts#L14-L16

[bajtos]

👎 for using an anonymous tuple. Let's define a new interface with named properties please.

export interface FindOrCreateResult<T extends Entity> {
  entity: T;
  wasCreated: boolean;
}

I am not sure what would be the best name for the boolean property. I find the short name "created" misleading, because "created" is usually set to a timestamp (creation time). Few alternatives that come to my mind:

• wasCreated
• isNew
• wasFound
• found

[bajtos]

On the second thought, I think I like found most.

[b-admike]

Good point! I was thinking about defining a type/interface too, but thought it might be a one off thing. I think found is reasonable 👍.

[bajtos]

Let's use reverse logic + early return to keep this method simpler & easier to reason about.

async findOrCreate(
  filter: Filter<T>,
  entity: DataObject<T>,
  options?: AnyObject | undefined,
): Promise<[T, boolean]> {
  const canRunAtomically = typeof this.dataSource.connector.findOrCreate === 'function';
  if (!canRunAtomically) {
    throw new Error('Method not implemented.');
    // FIXME add machine-readable `err.code`
  }

  const result = await ensurePromise(
    this.modelClass.findOrCreate(filter as legacy.Filter, entity, options),
  );
  return [this.toEntity(result[0]), result[1]];
}

[bajtos]

I don't see how does this test relates for findOrCreate functionality, could you please help me to understand?

[b-admike]

I copied the tests from DefaultCrudRepository unit test file because I was worried code coverage would drop if I didn't have them and since we extend from it.

[bajtos]

Well, as far as I understand code coverage, it counts lines of source code. As long as there is a test executing the source code behind DefaultCrudRepository in the file legacy-juggler-bridge.ts, it does not matter whether this test creates an instance of DefaultCrudRepository directly or an instance of a class inheriting from this Repository implementation.

[b-admike]

Ah yeah that is correct, I misunderstood how it worked. Thanks for the explanation, I will remove them and have assertions that you proposed in #1974 (comment).

[bajtos]

Isn't this scenario already covered by other tests? Why are we duplicating them here?

[b-admike]

I copied the tests from DefaultCrudRepository unit test file because I was worried code coverage would drop if I didn't have them and since we extend from it.

[bajtos]

These operations are already covered by DefaultCrudRepository tests, please don't duplicate them again.

If we want to verify that AtomicCrudRepository uses the same CRUD implementation as DefaultCrudRepository, then just check for function equality.

Here is what I have in my mind, I hope it will work as I expect:

const repo = new DefaultAtomicCrudRepository(Note, ds); 
expect(repo.create).to.equal(DefaultCrudRepository.prototype.create);

[bajtos]

how can we test a connector that doesn't have findOrCreate?

See the existing test suite in juggler for inspiration:
https://github.com/strongloop/loopback-datasource-juggler/blob/f0a6bd146b7ef2f987fd974ffdb5906cf6a584db/test/memory.test.js#L904-L923

ds.connector.findOrCreate = false;

I think the following approach is a little bit more clean, but IDK if it works:

ds.connector.findOrCreate = undefined;

[b-admike]

Awesome, thanks for the link @bajtos!

[b-admike]

Closing as rejected in favour of #2005 (see #1956 (comment)).