guardian icon indicating copy to clipboard operation
guardian copied to clipboard

Avoid `domain` package to follow odpf standard

Open mabdh opened this issue 3 years ago • 0 comments

Summary ODPF project structure standard prefer to follow domain-driven structure where each domain is located inside its domain main package inside core package.

However in guardian, we still have a domain package that holds all domains. With the approach like this, it is more likely for the developers to not separate the concern well in the further development considering there is no restriction when adding a functionality for a domain. This can also lead to more complex spaghetti code that mixed logic between several domains.

We need to follow ODPF project structure standard slowly to have a design restriction so we can be more mindful when developing a new functionalities. I am also hoping this will simplify the code base and increase the readability & predictability.

Proposed solution

I have analyzed the codebase and found there are several domains defined: appeal, policy, approval, provider, resource.

  1. Move resource to its own package The resource domain is the "lowest-level" domain that does not depend to any other domain. We can safely move this to its own domain called resource.

  2. provider package The provider struct does not depend on any domain except for a resource. It is possible to move this to its own package. However the provider service depends on the Appeal struct, this makes provider highly coupled with appeal. There are two options.

  • Option 1
    • Move ValidateAppeal to appeal domain
    • Currently the plugins/providers depend on appeal too as its argument. Instead of doing this, It is better for the provider plugin to have its own payload struct as a new abstraction and then appeal package translates appeal to the defined struct. This will avoid provider and provider plugins to have dependency to appeal.
  • Option 2
    • Move provider struct to provider package and provider service to a providerproxy package. This might be the fastest solution to decouple in the meantime since there is no logic changed.
  1. Highly coupling between appeal, policy, and approval
  • The least destructive attempt to move from domain package for these packages are to club them into a single package called appeal.
  • However we can do further more, decoupling approval and appeal & policy. The problem with current design is appeal depends on several approvals but approval depend on appeal in our domain. We can make remove appeal from approval to break this circular dependencies. However approval grpc response still needs appeal as a part of its response. We could resolve this appeal in the handler layer using appealService

mabdh avatar Jun 29 '22 03:06 mabdh