<div dir="ltr"><div><div><div><div>Thanks Konrad and Thomas for your time and inputs. I will create a Github issue for tracking the changes made regarding layering.<br></div><div><br>@Thomas, thank you for elaborating the content with problem-solution. When we say a system-layer., is it a bunch of related packages ? Or a layer is a package.<br></div><div><br></div>From the content above, what I understand is, we should have three main system-related packages.<br></div>1. Interfaces (can import from infra)<br></div>2. Impl (Implementing interfaces from above package, should import from interfaces)<br></div>3.
Infra (All infrastructure related stuff e.g> Lifecycle,
GormTransactionSupport, must not import from interfaces/impl. In case,
it needs to import from interface, then we need to think is it really
infrastructure item)<br><div><br>4. Other business related packages e.g> remoteworkitem, account, login etc.<br></div><div><br></div><div>As a first step I will move Lifecycle to infra package (outside models).<br><br></div><div>@Thomas, is <a href="https://github.com/almighty/almighty-core/issues/110">this (#110)</a> existing architecture issue for the same ? If yes, I will update the same.<br></div><div><br></div><div><br><br></div><div>Thanks,<br></div>Pranav.<br><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Oct 10, 2016 at 3:03 PM, Thomas Mäder <span dir="ltr"><<a href="mailto:tmader@redhat.com" target="_blank">tmader@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<p>Hi Pranav,</p>
<p>I think your question touches upon an unresolved problem we have
with our system layering. We have discussed this before when I
wrote the first repository interfaces, but haven't really resolved
the issue at the time. Let's look at what kinds of code we have:</p>
<ul>
<li>Repository interfaces<br>
They define an interface for persistence in terms of application
level objects. The repository interfaces are not supposed to be
gorm specific. The idea is to be able to replace the
implementation in tests and to avoid coupling consumers to gorm.</li>
<li>Repository implementations<br>
They implement the repository implementations in a gorm-specific
way. They are free to convert app layer objects to a different
form. For example, we store work items as JSON in the db, which
only has very few data types (number, string, bool, etc.). The
repository implementations convert and validate the work item
fields according to a given work item type. <br>
</li>
<li>Gorm support stuff <br>
Stuff like models.Lifecycle, GormTransactionSupport, etc. This
is shared infrastructure for implementing repositories.</li>
</ul>
<p>Now that we have an overview, what problems can we run into and
how do we solve them?<br>
</p>
<ul>
<li>Problem 1: shared repository infrastructure in models package<br>
Solution: -> move infrastructure to a separate package</li>
<li>Problem 2: false cycle<br>
This problem can occur if we have multiple repositories in the
same package and have pairwise inverse dependencies: Think of
two packages foo and bar. We have repository foo.X depend on
bar.Y and repository bar.M depend on foo.N. Now we have a
package cycle, but it is not a REAL cycle in the code, we just
co-located two independent repositories in the same package<br>
Solution: -> separate repositories into different packages<br>
</li>
<li>Problem 3: real cycle<br>
In real life, there ARE cases where cyclic dependencies make
sense. Imaging an event service and a logging service: the event
service wants to log things, and the logging service wants to
send events. You usually need some kind of setup protocol to get
things going, but it's possible. The problem in our case is that
the compiler will complain if the two repositories are in
separate packages.<br>
Solution: -> move the two repository interfaces to a common
package and only depend on the interface of the other repository
in the implementation.</li><span class="HOEnZb"><font color="#888888">
</font></span></ul><span class="HOEnZb"><font color="#888888">
<p>/Thomas<br>
</p></font></span><div><div class="h5">
<br>
<div>On 10/08/2016 05:25 AM, Pranav Gore
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div>
<div>Hi Team Core,<br>
<br>
<b>Context/Background:-</b><br>
I am working on <a href="https://github.com/almighty/almighty-core/issues/244" target="_blank">#244</a>. The task is to do <span style="background-color:rgb(255,255,255)">`fields["system.creator"]=<wbr>currentLoggedInUser</span>`.<br>
<br>
There, I want to call a method Locate from package "Token"
which returns current logged in user.<br>
While doing so I am facing cyclic import problem.<br>
<br>
<b>Reason:-</b> Work_item_repo is in package "models", it
wants to call Locate method from package "token".<br>
But "token" pachakge imports "account" package and "account"
package imports "models".<br>
Hence "models" package is unable to import "token" =>
cyclic dependency.<br>
<br>
<b>Debugging:-</b> When I was looking into "account"
package, what exactly it needs from "models" package is ONLY
"models.LifeCycle" and nothing else as of now.<br>
<br>
<b>Solutions:-</b> One quick possible/feasible solution is
to put models.LifeCycle into different new package of its
own, refactor and move on (IMO we should do this as first
step).<br>
<br>
Another is to write a middleware(as mentioned in <a href="https://github.com/almighty/almighty-core/issues/244" target="_blank">#244</a>)
for injecting loggedInUser which will be in package
"middleware" and avoid cyclic import, <br>
<br>
Or redesigning to decouple models package into smaller
chunks ?<br>
</div>
<div><br>
PFA- I have attached output of `go list -f` on account,
models, token packages.<br>
<br>
Any input on this will be helpful :)<br>
</div>
<div><br>
<br>
</div>
Thanks,<br>
</div>
Pranav<br>
</div>
</blockquote>
<br>
</div></div></div>
</blockquote></div><br></div>