spring-projects / spring-projects/spring-data-relational

Circular dependency between logic layers

Open
#1,435 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

type: enhancement
Dominant language
Java
Stars
827
Forks
394
PR merge metrics
No merged PRs in 30d

Description

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing DataAccessStrategy, EntityRowMapper, MapEntityRowMapper, and ReadingContext#resolveRelation, including their interactions with the spring-jdbc API. Map the four proposed responsibilities—SQL creation, ParameterSource construction, execution, and row mapping—and identify the smallest separable task. Done should mean the layer dependency is removed and the relation-fetching behavior is covered by appropriate tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, spring
Domain
backend, database
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.