International-Data-Spaces-Association / ids-specification

The Dataspace Protocol is a set of specifications designed to facilitate interoperable data sharing between entities governed by usage control and based on Web technologies. These specifications define the schemas and protocols required for entities to publish data, negotiate Agreements, and access data in a data space
https://docs.internationaldataspaces.org/dataspace-protocol/
Apache License 2.0
26 stars 14 forks source link

feat: Remove unnecessary dataset id references #193

Closed jimmarino closed 5 months ago

jimmarino commented 6 months ago

This PR removes unnecessary use of the dsp:dataset attribute from the ContractOfferMessage and ContractRequestMessage but leaves it in the CatalogRequestMessage. For the former messages, the ODRL policy target attribute is used and the restriction to not allow that attribute in policies has been removed.

A new restriction is introduced to not allow target attributes on policy constraints as it does not make sense in our context but is permitted in ODRL.

Related Issues

Closes #192

matgnt commented 6 months ago

To my understanding this would introduce a redundant dataset id in the catalog, correct? 1 in the dataset itself and 1 in the enclosed offer->target

ndr-brt commented 6 months ago

To my understanding this would introduce a redundant dataset id in the catalog, correct? 1 in the dataset itself and 1 in the enclosed offer->target

yes, but that will avoid additional fields in the messages to specify the dataset: the offer will be self-contained and already able to tell to what dataset it's referring to. Same will be with the agreement.

Furthermore, not putting the target in the policy will be a violation of:

defining target at a root level is a way to tell "every rule has this target": https://www.w3.org/TR/odrl-model/#composition-compact

jimmarino commented 6 months ago

To my understanding this would introduce a redundant dataset id in the catalog, correct? 1 in the dataset itself and 1 in the enclosed offer->target

It would remove the redundant dataset property in the ContractOfferMessage and ContractRequestMessage. It would add the target attribute back to an Offer which must match the enclosing dataset id. This would make the Offer symmetrical with other policies. However, it would still allow referencing policies by URI and adding a target (so the referenced policy can be used generically):

"hasPolicy": {
   "@type": "Policy",
   "@id": "https://test.com/some-policy.json",
   "target": "datasetid"
}
jimmarino commented 6 months ago

Also the https://github.com/International-Data-Spaces-Association/ids-specification/blob/main/model/model.md file needs to be updated

I don't see where it needs to be updated beyond the changes I made in the PR. Did I miss something?

ndr-brt commented 6 months ago

Also the https://github.com/International-Data-Spaces-Association/ids-specification/blob/main/model/model.md file needs to be updated

I don't see where it needs to be updated beyond the changes I made in the PR. Did I miss something?

https://github.com/International-Data-Spaces-Association/ids-specification/blob/main/model/model.md?plain=1#L70

jimmarino commented 6 months ago

Also the https://github.com/International-Data-Spaces-Association/ids-specification/blob/main/model/model.md file needs to be updated

I don't see where it needs to be updated beyond the changes I made in the PR. Did I miss something?

https://github.com/International-Data-Spaces-Association/ids-specification/blob/main/model/model.md?plain=1#L70

Isn't that changed in the PR?

https://github.com/International-Data-Spaces-Association/ids-specification/pull/193/files#diff-759257c54564e8398e104d8ac97c57863a742dd341e12a45a9e8e492475b7398

ndr-brt commented 6 months ago

oh right, I overlooked that, sorry

matgnt commented 5 months ago

Please do not merge yet. We need to discuss this further, e.g. my comment in the original issu: https://github.com/International-Data-Spaces-Association/ids-specification/issues/192#issuecomment-1885135591

ssteinbuss commented 5 months ago

@jimmarino please revise/ turn back to old status as discussed