Skip to content

Commit

Permalink
fix!: improve DevEx by further constraining types of some ops options
Browse files Browse the repository at this point in the history
FindAllOptions.sortBy and filters attribute from FindOneOptions, FindAllOptions, and DeleteAllOptions used to be of type 'any' . Now devs will experience TS errors when they do not provide valid values.
  • Loading branch information
Josuto committed Jun 1, 2024
1 parent 6dbbad8 commit f884fd7
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 31 deletions.
9 changes: 6 additions & 3 deletions src/mongoose.repository.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import mongoose, {
Connection,
FilterQuery,
HydratedDocument,
Model,
UpdateQuery,
Expand Down Expand Up @@ -77,7 +78,9 @@ export abstract class MongooseRepository<T extends Entity & UpdateQuery<T>>
}

/** @inheritdoc */
async findOne<S extends T>(options?: FindOneOptions): Promise<Optional<S>> {
async findOne<S extends T>(
options?: FindOneOptions<S>,
): Promise<Optional<S>> {
const document = await this.entityModel
.findOne(options?.filters ?? undefined)
.session(options?.session ?? null)
Expand All @@ -86,7 +89,7 @@ export abstract class MongooseRepository<T extends Entity & UpdateQuery<T>>
}

/** @inheritdoc */
async findAll<S extends T>(options?: FindAllOptions): Promise<S[]> {
async findAll<S extends T>(options?: FindAllOptions<S>): Promise<S[]> {
if (options?.pageable?.pageNumber && options?.pageable?.pageNumber < 0) {
throw new IllegalArgumentException(
'The given page number must be a positive number',
Expand All @@ -102,7 +105,7 @@ export abstract class MongooseRepository<T extends Entity & UpdateQuery<T>>
const pageNumber = options?.pageable?.pageNumber ?? 0;
try {
const documents = await this.entityModel
.find(options?.filters)
.find(options?.filters as FilterQuery<S>)
.skip(pageNumber > 0 ? (pageNumber - 1) * offset : 0)
.limit(offset)
.sort(options?.sortBy)
Expand Down
2 changes: 1 addition & 1 deletion src/mongoose.transactional-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export abstract class MongooseTransactionalRepository<
}

/** @inheritdoc */
async deleteAll(options?: DeleteAllOptions): Promise<number> {
async deleteAll<S extends T>(options?: DeleteAllOptions<S>): Promise<number> {
return await runInTransaction(
async (session: ClientSession) =>
(await this.entityModel.deleteMany(options?.filters, { session }))
Expand Down
4 changes: 2 additions & 2 deletions src/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ export interface Repository<T extends Entity> {
* @returns {Promise<Optional<S>>} the entity or null.
* @throws {IllegalArgumentException} if the given `filters` parameter is `undefined` or `null`.
*/
findOne: <S extends T>(options?: FindOneOptions) => Promise<Optional<S>>;
findOne: <S extends T>(options?: FindOneOptions<S>) => Promise<Optional<S>>;

/**
* Finds all entities.
* @param {FindAllOptions=} options (optional) search operation options.
* @returns {Promise<S[]>} all entities.
* @throws {IllegalArgumentException} if the given `options` specifies an invalid parameter.
*/
findAll: <S extends T>(options?: FindAllOptions) => Promise<S[]>;
findAll: <S extends T>(options?: FindAllOptions<S>) => Promise<S[]>;

/**
* Saves (insert or update) an entity.
Expand Down
2 changes: 1 addition & 1 deletion src/transactional-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,5 @@ export interface TransactionalRepository<T extends Entity>
* @returns {number} the number of deleted entities.
* @see {@link DeleteAllOptions}
*/
deleteAll: (options?: DeleteAllOptions) => Promise<number>;
deleteAll: <S extends T>(options?: DeleteAllOptions<S>) => Promise<number>;
}
25 changes: 15 additions & 10 deletions src/util/operation-options.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { FilterQuery } from 'mongoose';
import { IllegalArgumentException } from './exceptions';
import { TransactionOptions } from './transaction';

Expand Down Expand Up @@ -44,15 +45,19 @@ export class Pageable {
}
}

export type SortOrder = {
[key: string]: 'asc' | 'desc' | 'ascending' | 'descending' | 1 | -1;
};

/**
* Specifies options for the `findAll` operation.
* @property {any=} filters (optional) some filters for the search.
* @property {any=} sortBy (optional) the sorting criteria for the search.
* @property {FilterQuery=} filters (optional) some filters for the search.
* @property {string | SortOrder=} sortBy (optional) the sorting criteria for the search.
* @property {Pageable=} pageable (optional) paging configuration.
*/
export type FindAllOptions = {
filters?: any;
sortBy?: any;
export type FindAllOptions<T> = {
filters?: FilterQuery<T>;
sortBy?: string | SortOrder;
pageable?: Pageable;
} & TransactionOptions;

Expand All @@ -64,8 +69,8 @@ export type FindByIdOptions = TransactionOptions;
/**
* Specifies options for the `findOne` operation;
*/
export type FindOneOptions = {
filters?: any;
export type FindOneOptions<T> = {
filters?: FilterQuery<T>;
} & TransactionOptions;

/**
Expand All @@ -80,10 +85,10 @@ export type SaveAllOptions = AuditOptions & TransactionOptions;

/**
* Specifies options for the `deleteAll` operation.
* @property {any=} filters (optional) a MongoDB query object to select the entities to be deleted.
* @property {FilterQuery=} filters (optional) a MongoDB query object to select the entities to be deleted.
*/
export type DeleteAllOptions = {
filters?: any;
export type DeleteAllOptions<T> = {
filters?: FilterQuery<T>;
} & TransactionOptions;

/**
Expand Down
37 changes: 24 additions & 13 deletions test/repository/book.repository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,9 @@ describe('Given an instance of book repository', () => {

describe('using a null filter', () => {
it('returns an arbitrary book', async () => {
const book = await bookRepository.findOne({ filters: null });
const book = await bookRepository.findOne({
filters: null as unknown as object,
});
expect(book.isPresent()).toBe(true);
expect([storedPaperBook, storedAudioBook]).toContainEqual(book.get());
});
Expand Down Expand Up @@ -277,7 +279,7 @@ describe('Given an instance of book repository', () => {

describe('and such a value refers to an existing field in some Book type', () => {
it('retrieves a list with all books matching the filter', async () => {
const filters = { __t: 'PaperBook' };
const filters = { title: 'Effective Java' };
const books = await bookRepository.findAll({ filters });
expect(books.length).toBe(1);
expect(books).toEqual([storedPaperBook]);
Expand All @@ -286,19 +288,29 @@ describe('Given an instance of book repository', () => {
});

describe('and providing a value for the sort parameter', () => {
describe('and such a value is invalid', () => {
it('throws an exception', async () => {
const sortBy = { title: 2 };
await expect(bookRepository.findAll({ sortBy })).rejects.toThrow(
IllegalArgumentException,
);
describe('and such a value is the name of a book property', () => {
it('retrieves an ordered list with books', async () => {
const books = await bookRepository.findAll({
sortBy: 'title',
});
expect(books.length).toBe(3);
expect(books).toEqual([storedBook, storedPaperBook, storedAudioBook]);
});
});

describe('and such a value is ascendent on a book property', () => {
it('retrieves an ordered list with books', async () => {
const books = await bookRepository.findAll({
sortBy: { title: 'asc' },
});
expect(books.length).toBe(3);
expect(books).toEqual([storedBook, storedPaperBook, storedAudioBook]);
});
});

describe('and such a value is valid', () => {
describe('and such a value is descendent on a book property', () => {
it('retrieves an ordered list with books', async () => {
const sortBy = { title: -1 };
const books = await bookRepository.findAll({ sortBy });
const books = await bookRepository.findAll({ sortBy: { title: -1 } });
expect(books.length).toBe(3);
expect(books).toEqual([storedAudioBook, storedPaperBook, storedBook]);
});
Expand Down Expand Up @@ -936,11 +948,10 @@ describe('Given an instance of book repository', () => {
describe('and providing a valid value for all optional parameters', () => {
it('retrieves an ordered list with books matching the filter', async () => {
const filters = { __t: ['PaperBook', 'AudioBook'] };
const sortBy = { title: -1 };
const pageable = { pageNumber: 1, offset: 1 };
const books = await bookRepository.findAll({
filters,
sortBy,
sortBy: { title: -1 },
pageable,
});
expect(books.length).toBe(1);
Expand Down
4 changes: 3 additions & 1 deletion test/repository/book.transactional-repository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,9 @@ describe('Given an instance of book repository', () => {

describe('that includes a null filter', () => {
it('deletes all books', async () => {
const deletedBooks = await bookRepository.deleteAll();
const deletedBooks = await bookRepository.deleteAll({
filters: null as unknown as object,
});
expect(deletedBooks).toBe(2);

const storedBooks = await bookRepository.findAll();
Expand Down

0 comments on commit f884fd7

Please sign in to comment.