Coupling issues

[titpetric]

As we're moving forward on #93, we detected a few issues with coupling:

1. main package coupling (messaging uses system/service,...),
2. DB object crossing package boundary (system uses DB handle from messaging),
3. transaction issues (messaging starts db transaction, invokes a transaction on system - possibly doesn't exist in real life, but there's no mechanism to prevent it either).

Several proposals to start solving these issues (all need to be implemented).

---

Proposal 1 (easy)

Under each microservice (crm, messaging, system), we have:

• repository
• service,
• rest, websocket...

I'm suggesting we create an internal package here, moving repository and service there.

As any APIs at this point are inaccessible from other packages, we solve coupling 2, and 3.

Difficulty: easy - mostly it's just modifying some imports, low hanging fruit.

---

Proposal 2 (medium)

Microservices like messaging and crm need access to user data from system:

Options:

1. have system/service with things like FindUser(uint64), FindUsers([]uint64);
2. same as 1), but define an internal/service

I'm leaning towards 2), as we're already using the root internal package and subpackages to share logic (permissions, rules, ...). Crossing from one microservice package into another shouldn't be allowed. The main question that will probablly decide this is responsibility - do we want the system microservice responsible for user data, or do we want to put this responsibility into internal.

The main challenge here is enforcing read-only access.

Difficulty: medium - requires refactoring existing API access and SQL queries over all microservices. Some violations can be detected with simple scripts (grep "sys_" in messaging, etc.).

---

Proposal 3 (medium - hard)

Internal (shared) APIs shouldn't have the DB object as any of the parameters. We shouldn't enable injection of a DB object from messaging into system. Any public shared API constructor that's accessible (New..., With, etc.) should retrieve the appropriate DB object internally.

Difficulty: this significantly modifies existing patterns for services (constructors/DI, context,...), it might be worth to plan this out in more detail, I think there are design improvements that can be made here.

---

Feel free to add on any proposals you may have.

[darh]

Proposal 1

That means putting moving all business logic into one pot. Strongly agaist.

Proposal 2

Removing user/FindById & types etc should be trivial to remove. Might have some frontend implications -- moving more data loading responsibilities to the frontend Services/repositories inside crm/messaging do not access any sys_* tables directly.

Additinal dependancy is permissions/rules -- both, CRM & Messaging have that hard dependancy.

Regarding cross-service-data-referencing (example: testing if user exists from messaging). We can start with trusting the source and then (asap) move to some kind of internal API between microservices?

Proposal 3

Not sure if I understand what is the problem here, can you clarify please.

[titpetric]

Proposal 1:

I think you misunderstood, business logic is still separated, but under crm/internal/..., messaging/internal/... so messaging wouldn't be able to import system/internal...

Basically, repository and service are moved one level deeper.

Proposal 2:

That's great - but at least some test from messaging reference sys_ tables currently. Definitely could stand to have a cleanup.

Proposal 3:

I was trying to find the example, but it seems we don't do that already. Effectively, it would look like importing system/repository from messages/repository, and passing along a DB object as an argument. So thankfully this is basically a NO-OP

[darh]

1) ack 2) ok 3) ok

[titpetric]

This has effectively been implemented. There is an open issue with system-cli, but I'm opening another issue for that.