Move empty tree id detection from Gitlab::GitalyClient::CommitService
Explanation
Interactions in the code can be described with a following diagram:
flowchart TD
Repository --> Git::Repository
Git::Repository --> Gitlab::GitalyClient::CommitService
where,
-
Repository
is a high level class that has access to the database and the project -
Git::Repository
represents a low level repository entity that knows how to work with Gitaly RPCs -
Gitlab::GitalyClient::CommitService
is a wrapper to send and receive RPC messages from Gitaly
Problem
When commit id is not provided to CommitDiff
then we fallback to empty tree id value.
Introduce empty_tree_id to correctly work with ... (!144494 - merged) changes this behavior to dynamically calculate empty tree id. However, this change breaks the boundaries between Repository
model and CommitService
.
The new diagram looks like this:
flowchart TD
Repository --> Git::Repository
Git::Repository --> Gitlab::GitalyClient::CommitService
Gitlab::GitalyClient::CommitService -. request empty_tree_id .-> Repository
Gitlab::GitalyClient::CommitService
have to request the correct empty tree value from Repository
. That introduces an unnecessary dependency between these two classes.
Proposal 1
Move empty tree fallback logic to Repository
model and pass calculated commit ids through Git::Repository
to Gitlab::GitalyClient::CommitService
via method arguments. That will remove the dependency between Repository
and Gitlab::GitalyClient::CommitService
classes.
Proposal 2
Move empty tree fallback logic to Gitaly.
For example, when leftSha
is missing Gitaly can automatically replace the empty value with a correct empty tree id for this repository.
That will allow to significantly simplify code on Rails side.