Skip to content
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

Adding updateMany and deleteMany Methods to Remult repository #221

Closed
meni3a opened this issue Aug 11, 2023 · 11 comments
Closed

Adding updateMany and deleteMany Methods to Remult repository #221

meni3a opened this issue Aug 11, 2023 · 11 comments

Comments

@meni3a
Copy link

meni3a commented Aug 11, 2023

Description:

I would like to propose an enhancement to Remult by adding updateMany and deleteMany methods to the repository interface, which can be implemented across various database types.
Currently, developers are required to perform these operations individually using their IDs. This feature request aims to streamline the process by allowing developers to update or delete multiple entities in a single query.

Benefits:

Improved Performance: With the ability to update or delete multiple entities in a single query, performance improvements can be realized, especially when dealing with large datasets. This reduces the overhead of making multiple round-trips to the database.

Simplified Code: The addition of updateMany and deleteMany methods would lead to more concise and readable code. Developers would no longer need to write repetitive code for iterating through each entity and performing updates or deletions one by one.

Proposed API:

interface Repository<entityType> {
    ...
    deleteMany(where?: EntityFilter<entityType>): Promise<void>;
    updateMany(where?: EntityFilter<entityType>, item: Partial<OmitEB<entityType>>): Promise<entityType>;
}
@noam-honig
Copy link
Collaborator

Thanks. How would you handle lifecycle events such as validation, saving etc...

Also, in the updateMany - you indicated that you want to return an entity, I think it makes sense to return void, or number of rows affected

@meni3a
Copy link
Author

meni3a commented Aug 13, 2023

Thanks. How would you handle lifecycle events such as validation, saving etc...

Also, in the updateMany - you indicated that you want to return an entity, I think it makes sense to return void, or number of rows affected

  1. In order to avoid breaking changes in the entity lifecycle, we can iterate through the rows and trigger the events one by one. The alternative will be to modify the event callback functions to receive an array of rows.

  2. Regarding the return type, I meant Promise<entityType[]> (an array of the modified rows). but maybe modifiedCount would be more efficient. If we want to take it a step forward we can create a dedicated type with multiple properties (matchedCount, modifiedCount, errors, etc)

@philhawthorn
Copy link

I wanted to add my perspective to this conversation if I may.

update/delete in bulk would be an amazing feature, and having lifecycle events to reflect bulk CUD would really complete it.

Lets say i have this contrived example where i want to call an external service prior to the record being saved:

async saving(stock: Stock, e: LifecycleEvent<Stock>) {
    const { oldSync } = e.fields.find('sync');
    if(!oldSync && stock.sync) {
        // user has toggled sync option from off->on
        const [ticker, price] = await callExternalService(stock.tickerSymbol;
        entity.price = price
    }
}

whereas it would be great if i could perform it in bulk as part of the lifecycle:

async saving(payload: { stock: Stock, e: LifecycleEvent<Stock> }[]) {
    async saving(payload: { stock: Stock, e: LifecycleEvent<Stock> }[]) {
        const m = new Map(payload.filter(item=>{
            const { oldSync } = e.fields.find('sync');
            // user has toggled sync option from off->on
            return !oldSync && item.stock.sync
        }).map((item) => [item.stock.tickerSymbol, item.stock]);

        for(const [tickerSymbol, result] of await callExternalService(m.keys())) {
            m.get(tickerSymbol).price = result;
        }
    }
}

in terms of return type, i personally think it would be useful to have a list of success/failures, e.g

type SavingResponse = { 
    id: any,
    success: boolean;
    error: string
}

So the whole thing might look like this:

@Entity("stocks", {
    async saving(payload: { stock: Stock, e: LifecycleEvent<Stock> }[]) {
        const m = new Map(payload.filter(item=>{
            const { oldSync } = e.fields.find('sync');
            // user has toggled sync option from off->on
            return !oldSync && item.stock.sync
        }).map((item) => [item.stock.tickerSymbol, item.stock]);

        for(const [tickerSymbol, result] of await callExternalService(m.keys())) {
            m.get(tickerSymbol).price = result;
        }
    }
})
export class Stock {
    @Fields.autoIncrement()
    id: number

    @Fields.number()
    price: number

    @Fields.string()
    tickerSymbol: string

    @Fields.boolean()
    sync: boolean = false
}

...
for(const result of await remult.repo(Stock).update([{id: 1, sync: true},{id: 2, sync: true},{id: 3, sync: true}])) {
    if(!result.success) {
        console.log(`Failed to get stock price for stock id ${result.id} with error ${result.error}`)
    }
}

(PS, I realise I can achieve all of this by structuring the code differently but am just trying to explain why i think it would be a useful feature)

@jycouet
Copy link
Collaborator

jycouet commented Jan 27, 2024

Let me add a small thing as well: transaction.
No matter how it will be done (1 query, multiples, server checks, ...) if there is one issue in on place, would be nice to be back as before.

// was my 2 cents 😉

@philhawthorn
Copy link

@jycouet maybe a additional boolean parameter to give the option to fail the whole batch if any in the batch fail?

@noam-honig
Copy link
Collaborator

We've implemented deleteMany and updateMany to be released in our next version. you can check it out now in the exp version:

npm i remult@exp

Please check it out and let me know if you have any comments.

Currently, it's defined as a transaction, so it all succeeds or all fails.

@meni3a
Copy link
Author

meni3a commented Mar 23, 2024

That's great! Is it possible to add it to the API as well?
I think it would be very useful, just like we currently have separate routes for GET by ID and GET by filter.

@noam-honig
Copy link
Collaborator

Working on it

@noam-honig
Copy link
Collaborator

Hi @meni3a I've created a new exp version with documentation of deleteMany etc... in the OpenApi(swagger) and support for delete or put with search paramters.
There is also a safeguard against no filter.

Can you check it out and see if it's ok?

npm i remult@exp

@meni3a
Copy link
Author

meni3a commented Mar 27, 2024

@noam-honig Looks good!

@noam-honig
Copy link
Collaborator

Released in version 0.26.0:
https://github.com/remult/remult/releases/tag/v0.26.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants