-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Repo class factory improvements #4949
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e84e6ff
36d2a1b
90930c0
ccca433
b8e6a04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,138 @@ | ||
| // Copyright IBM Corp. 2020. All Rights Reserved. | ||
| // Node module: @loopback/repository | ||
| // This file is licensed under the MIT License. | ||
| // License text available at https://opensource.org/licenses/MIT | ||
|
|
||
| import {expect} from '@loopback/testlab'; | ||
| import { | ||
| AnyObject, | ||
| Count, | ||
| CrudRepository, | ||
| DataObject, | ||
| DefaultCrudRepository, | ||
| DefaultKeyValueRepository, | ||
| defineEntityCrudRepositoryClass, | ||
| defineKeyValueRepositoryClass, | ||
| defineRepositoryClass, | ||
| Entity, | ||
| Filter, | ||
| juggler, | ||
| model, | ||
| Model, | ||
| property, | ||
| Where, | ||
| } from '../../..'; | ||
|
|
||
| describe('RepositoryClass builder', () => { | ||
| describe('defineRepositoryClass', () => { | ||
| it('should generate custom repository class', async () => { | ||
| const AddressRepository = defineRepositoryClass< | ||
| typeof Address, | ||
| DummyCrudRepository<Address> | ||
| >(Address, DummyCrudRepository); | ||
| // `CrudRepository.prototype.find` is inherited | ||
| expect(AddressRepository.prototype.find).to.be.a.Function(); | ||
| // `DummyCrudRepository.prototype.findByTitle` is inherited | ||
| expect(AddressRepository.prototype.findByTitle).to.be.a.Function(); | ||
| expect(AddressRepository.name).to.equal('AddressRepository'); | ||
| expect(Object.getPrototypeOf(AddressRepository)).to.equal( | ||
| DummyCrudRepository, | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe('defineEntityCrudRepositoryClass', () => { | ||
| it('should generate entity CRUD repository class', async () => { | ||
| const ProductRepository = defineEntityCrudRepositoryClass(Product); | ||
|
|
||
| expect(ProductRepository.name).to.equal('ProductRepository'); | ||
| expect(ProductRepository.prototype.find).to.be.a.Function(); | ||
| expect(ProductRepository.prototype.findById).to.be.a.Function(); | ||
| expect(Object.getPrototypeOf(ProductRepository)).to.equal( | ||
| DefaultCrudRepository, | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe('defineKeyValueRepositoryClass', () => { | ||
| it('should generate key value repository class', async () => { | ||
| const ProductRepository = defineKeyValueRepositoryClass(Product); | ||
|
|
||
| expect(ProductRepository.name).to.equal('ProductRepository'); | ||
| expect(ProductRepository.prototype.get).to.be.a.Function(); | ||
| expect(Object.getPrototypeOf(ProductRepository)).to.equal( | ||
| DefaultKeyValueRepository, | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| @model() | ||
| class Product extends Entity { | ||
| @property({id: true}) | ||
| id: number; | ||
|
|
||
| @property() | ||
| name: string; | ||
| } | ||
|
|
||
| @model() | ||
| class Address extends Model { | ||
| @property() | ||
| street: string; | ||
|
|
||
| @property() | ||
| city: string; | ||
|
|
||
| @property() | ||
| state: string; | ||
| } | ||
|
|
||
| class DummyCrudRepository<M extends Model> implements CrudRepository<M> { | ||
| constructor( | ||
| private modelCtor: typeof Model & {prototype: M}, | ||
| private dataSource: juggler.DataSource, | ||
| ) {} | ||
| create( | ||
| dataObject: DataObject<M>, | ||
| options?: AnyObject | undefined, | ||
| ): Promise<M> { | ||
| throw new Error('Method not implemented.'); | ||
| } | ||
| createAll( | ||
| dataObjects: DataObject<M>[], | ||
| options?: AnyObject | undefined, | ||
| ): Promise<M[]> { | ||
| throw new Error('Method not implemented.'); | ||
| } | ||
| find( | ||
| filter?: Filter<M> | undefined, | ||
| options?: AnyObject | undefined, | ||
| ): Promise<(M & {})[]> { | ||
| throw new Error('Method not implemented.'); | ||
| } | ||
| updateAll( | ||
| dataObject: DataObject<M>, | ||
| where?: Where<M> | undefined, | ||
| options?: AnyObject | undefined, | ||
| ): Promise<Count> { | ||
| throw new Error('Method not implemented.'); | ||
| } | ||
| deleteAll( | ||
| where?: Where<M> | undefined, | ||
| options?: AnyObject | undefined, | ||
| ): Promise<Count> { | ||
| throw new Error('Method not implemented.'); | ||
| } | ||
| count( | ||
| where?: Where<M> | undefined, | ||
| options?: AnyObject | undefined, | ||
| ): Promise<Count> { | ||
| throw new Error('Method not implemented.'); | ||
| } | ||
|
|
||
| // An extra method to verify it's available for the defined repo class | ||
| findByTitle(title: string): Promise<M[]> { | ||
| throw new Error('Method not implemented.'); | ||
| } | ||
| } | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,170 @@ | ||
| // Copyright IBM Corp. 2020. All Rights Reserved. | ||
| // Node module: @loopback/repository | ||
| // This file is licensed under the MIT License. | ||
| // License text available at https://opensource.org/licenses/MIT | ||
|
|
||
| import assert from 'assert'; | ||
| import {PrototypeOf} from './common-types'; | ||
| import {Entity, Model} from './model'; | ||
| import { | ||
| DefaultCrudRepository, | ||
| DefaultKeyValueRepository, | ||
| juggler, | ||
| Repository, | ||
| } from './repositories'; | ||
|
|
||
| /** | ||
| * Signature for a Repository class bound to a given model. The constructor | ||
| * accepts only the dataSource to use for persistence. | ||
| * | ||
| * `define*` functions return a class implementing this interface. | ||
| * | ||
| * @typeParam M - Model class | ||
| * @typeParam R - Repository class/interface | ||
| */ | ||
| export interface NamedRepositoryClass< | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no need to have specialized types for KeyValue and CRUD repositories, we can use a generic Repository type as the starting point. I am not comfortable using I was looking for a name that would describe the fact that the repository class is already bound to a given model.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's What about
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should take another angle. My intention is to distinguish between a generic repository class that works for any Model/Entity and a repository class that's already specialized for (bound to) a given Model/Entity. In that light, perhaps |
||
| M extends Model, | ||
| R extends Repository<M> | ||
| > { | ||
| /** | ||
| * The constructor for the generated repository class | ||
| * @param dataSource - DataSource object | ||
| */ | ||
| new (dataSource: juggler.DataSource): R; | ||
| prototype: R; | ||
| } | ||
|
|
||
| /** | ||
| * Signature for repository classes that can be used as the base class for | ||
| * `define*` functions. The constructor of a base repository class accepts | ||
| * the target model constructor and the datasource to use. | ||
| * | ||
| * `define*` functions require a class implementing this interface on input. | ||
| * | ||
| * @typeParam M - Model class constructor, e.g `typeof Model`. | ||
| * **❗️IMPORTANT: The type argument `M` is describing the model constructor type | ||
| * (e.g. `typeof Model`), not the model instance type (`Model`) as is the case | ||
| * in other repository-related types. The constructor type is required | ||
| * to support custom repository classes requiring a Model subclass in the | ||
| * constructor arguments, e.g. `Entity` or a user-provided model.** | ||
| * | ||
| * @typeParam R - Repository class/interface | ||
| */ | ||
| export interface BaseRepositoryClass< | ||
| M extends typeof Model, | ||
| R extends Repository<PrototypeOf<M>> | ||
| > { | ||
| /** | ||
| * The constructor for the generated repository class | ||
| * @param modelClass - Model class | ||
| * @param dataSource - DataSource object | ||
| */ | ||
| new (modelClass: M, dataSource: juggler.DataSource): R; | ||
| prototype: R; | ||
| } | ||
|
|
||
| /** | ||
| * Create (define) a repository class for the given model. | ||
| * | ||
| * See also `defineEntityCrudRepositoryClass` and `defineKeyValueRepositoryClass` | ||
| * for convenience wrappers providing repository class factory for the default | ||
| * CRUD and KeyValue implementations. | ||
| * | ||
| * **❗️IMPORTANT: The compiler (TypeScript 3.8) is not able to correctly infer | ||
| * generic arguments `M` and `R` from the class constructors provided in | ||
| * function arguments. You must always provide both M and R types explicitly.** | ||
| * | ||
| * @example | ||
| * | ||
| * ```ts | ||
| * const AddressRepository = defineRepositoryClass< | ||
| * typeof Address, | ||
| * DefaultEntityCrudRepository< | ||
| * Address, | ||
| * typeof Address.prototype.id, | ||
| * AddressRelations | ||
| * >, | ||
| * >(Address, DefaultCrudRepository); | ||
| * ``` | ||
| * | ||
| * @param modelClass - A model class such as `Address`. | ||
| * @param baseRepositoryClass - Repository implementation to use as the base, | ||
| * e.g. `DefaultCrudRepository`. | ||
| * | ||
| * @typeParam M - Model class constructor (e.g. `typeof Address`) | ||
| * @typeParam R - Repository class (e.g. `DefaultCrudRepository<Address, number>`) | ||
| */ | ||
| export function defineRepositoryClass< | ||
| M extends typeof Model, | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly here, we need |
||
| R extends Repository<PrototypeOf<M>> | ||
| >( | ||
| modelClass: M, | ||
| baseRepositoryClass: BaseRepositoryClass<M, R>, | ||
| ): NamedRepositoryClass<PrototypeOf<M>, R> { | ||
| const repoName = modelClass.name + 'Repository'; | ||
| const defineNamedRepo = new Function( | ||
| 'ModelCtor', | ||
| 'BaseRepository', | ||
| `return class ${repoName} extends BaseRepository { | ||
| constructor(dataSource) { | ||
| super(ModelCtor, dataSource); | ||
| } | ||
| };`, | ||
| ); | ||
|
|
||
| const repo = defineNamedRepo(modelClass, baseRepositoryClass); | ||
| assert.equal(repo.name, repoName); | ||
| return repo; | ||
| } | ||
|
|
||
| /** | ||
| * Create (define) an entity CRUD repository class for the given model. | ||
| * This function always uses `DefaultCrudRepository` as the base class, | ||
| * use `defineRepositoryClass` if you want to use your own base repository. | ||
| * | ||
| * @example | ||
| * | ||
| * ```ts | ||
| * const ProductRepository = defineEntityCrudRepositoryClass< | ||
| * Product, | ||
| * typeof Product.prototype.id, | ||
| * ProductRelations | ||
| * >(Product); | ||
| * ``` | ||
| * | ||
| * @param entityClass - An entity class such as `Product`. | ||
| * | ||
| * @typeParam E - An entity class | ||
| * @typeParam IdType - ID type for the entity | ||
| * @typeParam Relations - Relations for the entity | ||
| */ | ||
| export function defineEntityCrudRepositoryClass< | ||
bajtos marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| E extends Entity, | ||
| IdType, | ||
| Relations extends object | ||
| >( | ||
| entityClass: typeof Entity & {prototype: E}, | ||
| ): NamedRepositoryClass<E, DefaultCrudRepository<E, IdType, Relations>> { | ||
| return defineRepositoryClass(entityClass, DefaultCrudRepository); | ||
| } | ||
|
|
||
| /** | ||
| * Create (define) a KeyValue repository class for the given entity. | ||
| * This function always uses `DefaultKeyValueRepository` as the base class, | ||
| * use `defineRepositoryClass` if you want to use your own base repository. | ||
| * | ||
| * @example | ||
| * | ||
| * ```ts | ||
| * const ProductKeyValueRepository = defineKeyValueRepositoryClass(Product); | ||
| * ``` | ||
| * | ||
| * @param modelClass - An entity class such as `Product`. | ||
| * | ||
| * @typeParam M - Model class | ||
| */ | ||
| export function defineKeyValueRepositoryClass<M extends Model>( | ||
| modelClass: typeof Model & {prototype: M}, | ||
| ): NamedRepositoryClass<M, DefaultKeyValueRepository<M>> { | ||
| return defineRepositoryClass(modelClass, DefaultKeyValueRepository); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So TypeScript cannot infer the generic types from the parameters in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make inference happy, I have to do the following:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TypeScript does not allow
typeof DummyCrudRepository<M>.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately not. I think we should report a bug to TypeScript (if it wasn't already reported).
IMO,
castDummyRepositorydoes not solve the problem of people forgetting to specify the repository type and relying on the compiler to infer it. Once they realize some extra effort is needed, then I think it's best to provide generic type parameters and there is no need to introducecast*Repositoryfunctions.