Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Relation Between Contract Definition, Access Policy and Contract Policy #4023

Closed
afonsocorreiactw opened this issue Mar 19, 2024 · 3 comments
Labels
triage all new issues awaiting classification

Comments

@afonsocorreiactw
Copy link

Feature Request

The Contract Definition entity and the Policy entity are not directly related, both at the application and persistence layer. Instead, a String ID bridges these entities.
The Object-Oriented way of relating these entities should be having a Policy accessPolicy and Policy contractPolicy within the ContractDefinition class.
At the persistence layer, Contract Definitions should have 2 many-to-one relations with Policy, creating the relation between them.
Moreover, additional business validation rules should be added for endpoints related to these entities, such as:

  • Contract Definition creation should guarantee the existence of the related policies (at the moment, a contract definition can be created with a accessPolicyId and/or contractPolicyId that don't map to any existing Policy)
  • Policy deletion should not be possible if attached to a Contract Definition

Which Areas Would Be Affected?

Application Domain Layer
Database Schema
Endpoint business validation rules

Why Is the Feature Desired?

This feature solidifies the domain layer of the application, following a Domain Driven Design approach. The Contract Definition would represent the real relation between it and the attached policies.
This feature removes inconsistencies with the stored data (Contract Definitions with policyIds to not existing policies)
This feature facilitates querying for the policies of a contract definition.

Details

Part of this issue was already discussed (#3693) and an issue opened (#3702). Although it seems to be added to Milestone 13, this issue still persists.

@github-actions github-actions bot added the triage all new issues awaiting classification label Mar 19, 2024
@paullatzelsperger
Copy link
Member

Hi @afonsocorreiactw. I think there is significant misunderstanding of a few of the core concepts of EDC. Entities in EDC, such as policies, as well as Assets can exist independently. This is by design and for several reasons. EDC in itself is not an application but a platform, consisting of more or less loosely coupled modules, so referring to it as an "application layer" is incorrect. EDC does not claim to - nor does it intend to - follow DDD, thus assuming it to be the architectural foundation of EDC is incorrect.

There are however several architectural principles and design patterns that EDC is based upon, and I suggest you familiarize yourself with them and the code base first.

Enforcing any sort of object relation at the persistence layer would be a huge impact on the code base without providing any real value.

However, validating several rules on ingress, i.e. management API, has been discussed numerous times, and I would not be generally opposed to it, but it seems to not have gained any real traction so far.

In closing I would also ask you to please follow our contribution guidelines, that means don't randomly open issues without having a discussion first. Specifically, if additional validation on API ingress is what you're after, then I would suggest to:

  • open a (focussed!) discussion first, outlining specific painpoints and their potential remediation. I cannot stress enough how important it is to motivate a change request. Simply saying "I think it would be better to..." isn't enough.
  • give committers some time to brood over it, reply etc.
  • if it gets upvoted, be ready to provide a sharply focused issue (or issues)
  • if you intend to contribute:
    • provide a written technical proposal first
    • then, submit a focused PR

PS: as I'm sure you are aware it is easily possible, even now, to replace the current management API, or parts of it, with your own custom one, or even just supplying additional validation rules.

@ndr-brt
Copy link
Member

ndr-brt commented Mar 19, 2024

The Contract Definition entity and the Policy entity are not directly related, both at the application and persistence layer. Instead, a String ID bridges these entities. The Object-Oriented way of relating these entities should be having a Policy accessPolicy and Policy contractPolicy within the ContractDefinition class. At the persistence layer, Contract Definitions should have 2 many-to-one relations with Policy, creating the relation between them. Moreover, additional business validation rules should be added for endpoints related to these entities, such as:

@afonsocorreiactw As a long term DDD adept I can't disagree more with your statement. There's no literature that forces the way everyone should structure their own "Aggregate Root" like a ContractDefinition could be defined, and, in fact, in a world where applications needs to be modular to be able to scale, I think it's important to define clear bounded contexts that are not coupled with each other.

Eventual "aggregated" service logic is strictly dependent to its context (e.g. ContractDefinitionResolver for catalog and ConsumerOfferResolver for negotiaton).

Regard the process in how to tackle this kind of discussion, I agree 100% with what @paullatzelsperger already said.

@afonsocorreiactw
Copy link
Author

afonsocorreiactw commented Mar 19, 2024

Hello @paullatzelsperger and @ndr-brt
Thank you for your replies.

I think there were some misconceptions regarding the goal of the Feature Request.
Nevertheless, I'll be closing this issue as I see this change goes against the core design of the EDC.

Regarding the process, I reviewed the contribution guidelines and will be sure to follow them for future contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage all new issues awaiting classification
Projects
None yet
Development

No branches or pull requests

3 participants