search

mathias21 / KtorEasy

Suggested Ktor architecture
GNU General Public License v3.0
260 stars 30 forks source link

[Improvement] Proposal for moving data related code away from Controller #4

Open sherviiin opened 3 years ago

sherviiin commented 3 years ago

I want to propose a BaseApi, which will handle querying of data: fun <T> dbQuery(block: () -> T): T

Currently, this is handled in BaseController. See this line.

By doing this, Controller won't be concerned about transactions and 'how' the data is fetched. That part will be the API duty to provide the requested data. I can make a PR for this.

mathias21 commented 3 years ago

Yeah, I'm aware about that. I'm not really happy about keeping transaction logic in controller. But in other hand, controller is the one executing the business logic so it will know when a rollback (and what to rollback) is needed. Moving that to the API level will add extra effort on controller to fire rollbacks from APIs in case that part of the business logic fails. Also it will add extra effort at API level to track which operation exactly you want to rollback. With the current implementation, you can open a dbQuery block at Controller level, so all that is inside that block will be rolled back if exception happens. Thoughts?

sherviiin commented 3 years ago

IMO the controller needs to verify if a request is valid before handing it over to API. So the controller can still throw logical exceptions, while API can throw exceptions that are related to DB in this case. If this doesn't address your concern we can discuss it further with a concrete example.

mathias21 commented 3 years ago

Yes, call verifications can happen before starting a transaction, but if you are reaching different APIs in the same controller method, it can end up executing first API call successfully and failing the second one. This is the scenario I'm describing where roll backs could be needed, so controller has to undo what was done in the previous successfully API calls. We cannot expect to have just one single call to one API. If you take transaction control out of controller logic, you need that extra logic implementing rollbacks. If you think your solution can work fine in unhappy scenarios with rollbacks needed, you can drop a gist example and I will be happy to discuss and merge it if it brings value ;)