Skip to content

Circular dependency between logic layers #1435

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

Open
mipo256 opened this issue Feb 25, 2023 · 1 comment
Open

Circular dependency between logic layers #1435

mipo256 opened this issue Feb 25, 2023 · 1 comment
Labels
type: enhancement A general enhancement

Comments

@mipo256
Copy link
Contributor

mipo256 commented Feb 25, 2023

I think we have a design problem in the framework

First, there is DataAccessStrategy, which acts like an orchestrator in general - it is responsible to delegate the SQL creation, construction of the ParameterSource for the SQL, and actual execution of the query via spring-jdbc module api. Then we have an EntityRowMapper to map the result of the ResultSet to POJO.

The problem is that EntityRowMapper (as well as MapEntityRowMapper) is calling findAllByPath of the DataAccessStrategy to fetch the dependencies of an aggregate. It is at least a circular depenedency between logic layers, and it brings up a question (even if we will not take into consideration the issue raised in #1434 and we will go with N SELECT queries instead of simple joins):

Do we really understand that the entity has a relation by the time of getting a response from the database? No. But that's what actually the code is doing right now - we check for the relation existence in the ReadingContext#resolveRelation method and at that time, we are creating second SQL query (why we do it here, and not earlier?) and fetching the request. Instead we can choose to not execute the second SELECT request in some rare OneToOne mapping cases (for instance we have OneToOne mapping and the id of child aggregate in the root entity is already absent, so there is no need to execute second SELECT - we will not find anything).

So I think we should clearly separate the layers, like:

  1. creation of SQL to fetch all of the data that we need (no matter by what means, select. subselect, join or whatever it is) - we have the relationship information before the actual requests to the DB. Moreover, if you are concerned about the performance for the construction of the possible second SQL request - you can cache it, because fetching the child aggregate from the parent will be done the same (for example now it is a second select - it will be the same, just root id different, but it does not matter).
  2. Creation ParameterSource(-es) for the SQL created on the previous step.
  3. The actual execution layer, seems delegation to spring-jdbc is just fine
  4. And then the mapping by RowMappers

I think there is a lot of work required for this to be done, so maybe we should consider to decompose this task. In any case, I really think this issue should be taken into account for the future of the project.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 25, 2023
@schauder schauder added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 27, 2023
@schauder
Copy link
Contributor

That is a good analysis of the situation and I agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants