-
-
Notifications
You must be signed in to change notification settings - Fork 752
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
feat(databases): Add support for fast bulk operations #2945
base: dove
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
|
@@ -6,7 +6,8 @@ import { | |
AdapterBase, | ||
AdapterServiceOptions, | ||
PaginationOptions, | ||
AdapterParams | ||
AdapterParams, | ||
AdapterQuery | ||
} from '@feathersjs/adapter-commons' | ||
import sift from 'sift' | ||
import { NullableId, Id, Params, Paginated } from '@feathersjs/feathers' | ||
|
@@ -28,10 +29,12 @@ const _select = (data: any, params: any, ...args: any[]) => { | |
return base(JSON.parse(JSON.stringify(data))) | ||
} | ||
|
||
export type MemoryAdapterParams<Q = AdapterQuery> = AdapterParams<Q, Partial<MemoryServiceOptions>> | ||
|
||
export class MemoryAdapter< | ||
Result = any, | ||
Data = Partial<Result>, | ||
ServiceParams extends Params = Params, | ||
ServiceParams extends MemoryAdapterParams = MemoryAdapterParams, | ||
PatchData = Partial<Data> | ||
> extends AdapterBase<Result, Data, PatchData, ServiceParams, MemoryServiceOptions<Result>> { | ||
store: MemoryServiceStore<Result> | ||
|
@@ -145,18 +148,18 @@ export class MemoryAdapter< | |
async _create(data: Partial<Data>[], params?: ServiceParams): Promise<Result[]> | ||
async _create(data: Partial<Data> | Partial<Data>[], _params?: ServiceParams): Promise<Result | Result[]> | ||
async _create( | ||
data: Partial<Data> | Partial<Data>[], | ||
_data: Partial<Data> | Partial<Data>[], | ||
params: ServiceParams = {} as ServiceParams | ||
): Promise<Result | Result[]> { | ||
if (Array.isArray(data)) { | ||
return Promise.all(data.map((current) => this._create(current, params))) | ||
} | ||
const payload = Array.isArray(_data) ? _data : [_data] | ||
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. To skip the extra processing for large arrays, we can add another check for const payload = Array.isArray(_data) ? _data : [_data]
const results = (params.bulk ? [] : payload).map((value) => {
const id = (value as any)[this.id] || this._uId++
const current = _.extend({}, value, { [this.id]: id })
return _select((this.store[id] = current), params, this.id)
})
return params.bulk ? [] : Array.isArray(_data) ? results : results[0] 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. It looks like we also need to address the scenario where we create a single record and also pass const david = await service.create(
{
name: 'David',
age: 3,
created: true
},
{ bulk: true }
) We don't want to return an empty array for the above scenario since that breaks the request-to-response plurality mapping of the service interface. So we probably ought to throw an error: if (!Array.isArray(_data) && params.bulk) {
throw new BadRequest()
} |
||
const results = payload.map((value) => { | ||
const id = (value as any)[this.id] || this._uId++ | ||
const current = _.extend({}, value, { [this.id]: id }) | ||
|
||
const id = (data as any)[this.id] || this._uId++ | ||
const current = _.extend({}, data, { [this.id]: id }) | ||
const result = (this.store[id] = current) | ||
return _select((this.store[id] = current), params, this.id) | ||
}) | ||
|
||
return _select(result, params, this.id) as Result | ||
return params.bulk ? [] : Array.isArray(_data) ? results : results[0] | ||
} | ||
|
||
async _update(id: Id, data: Data, params: ServiceParams = {} as ServiceParams): Promise<Result> { | ||
|
@@ -202,11 +205,12 @@ export class MemoryAdapter< | |
...params, | ||
query | ||
}) | ||
const results = entries.map(patchEntry) | ||
|
||
return entries.map(patchEntry) | ||
return params.bulk ? [] : results | ||
} | ||
|
||
return patchEntry(await this._get(id, params)) // Will throw an error if not found | ||
return params.bulk ? [] : patchEntry(await this._get(id, params)) // Will throw an error if not found | ||
} | ||
|
||
async _remove(id: null, params?: ServiceParams): Promise<Result[]> | ||
|
@@ -225,7 +229,9 @@ export class MemoryAdapter< | |
query | ||
}) | ||
|
||
return Promise.all(entries.map((current: any) => this._remove(current[this.id] as Id, params))) | ||
entries.forEach((current: any) => delete this.store[(current as any)[this.id]]) | ||
|
||
return params.bulk ? [] : entries | ||
} | ||
|
||
const entry = await this._get(id, params) | ||
|
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.
Maybe we can update
allowMulti
to acceptcontext.id
and have a centralized place to throw an error wheneverparams.bulk === true
anddata
is an objectcontext.id
is null