ids-specification
ids-specification copied to clipboard
Issues in JSON-Schema Definition
Hi,
I am currently going in detail through JSON-Schema Definitions and I found some inconsistencies on these definitions. I am not sure how these were created - manually or generated. Please triage them and if necessary fix them:
- [x] Typo: compare https://creativecommons.org/schema.rdf https://github.com/International-Data-Spaces-Association/ids-specification/blob/524039dcf1d52eceb9acf00c90c7a237c97afd95/negotiation/message/schema/contract-schema.json#L291
- [ ] Not defined: https://github.com/International-Data-Spaces-Association/ids-specification/blob/524039dcf1d52eceb9acf00c90c7a237c97afd95/negotiation/message/schema/contract-schema.json#L330
- [x] Differs from ODRL definition: see https://www.w3.org/ns/odrl/2/ODRL22.ttl https://github.com/International-Data-Spaces-Association/ids-specification/blob/524039dcf1d52eceb9acf00c90c7a237c97afd95/negotiation/message/schema/contract-schema.json#L326
- [x] JSON-Schema Reference instead of a definition name: https://github.com/International-Data-Spaces-Association/ids-specification/blob/524039dcf1d52eceb9acf00c90c7a237c97afd95/negotiation/message/schema/contract-schema.json#L105
- [x]
@target
instead ofodrl:target
: https://github.com/International-Data-Spaces-Association/ids-specification/blob/84101623fa57d347f5440932a0a31f881e682d57/negotiation/message/schema/contract-schema.json#L156 - [x]
Agreement
recursively referring toAgreement
(to what purpose? same agreement, but different permissions?): https://github.com/International-Data-Spaces-Association/ids-specification/blob/84101623fa57d347f5440932a0a31f881e682d57/negotiation/message/schema/contract-schema.json#L118 https://github.com/International-Data-Spaces-Association/ids-specification/blob/84101623fa57d347f5440932a0a31f881e682d57/negotiation/message/schema/contract-schema.json#L21 - [ ] References to
odrl:prohibition
without definition of it: https://github.com/International-Data-Spaces-Association/ids-specification/blob/84101623fa57d347f5440932a0a31f881e682d57/negotiation/message/schema/contract-schema.json#L97 https://github.com/International-Data-Spaces-Association/ids-specification/blob/84101623fa57d347f5440932a0a31f881e682d57/negotiation/message/schema/contract-schema.json#L147 - [ ] The
dspace:timestamp
is basically a RFC3339 timestamp: https://github.com/International-Data-Spaces-Association/ids-specification/blob/84101623fa57d347f5440932a0a31f881e682d57/negotiation/message/schema/contract-schema.json#L134- Is there really a need for B.C. timestamps?
- 24:00:00 is not represented in the RFC3339
- Leap seconds are not represented, compare RFC3339: Section 5.6. Internet Date/Time Format and RFC3339: Appendix D. Leap Seconds
- [x]
AbstractPolicyRule
does not containodrl:target
: https://github.com/International-Data-Spaces-Association/ids-specification/blob/84101623fa57d347f5440932a0a31f881e682d57/negotiation/message/schema/contract-schema.json#L72
I will extend the list when I find more inconsistencies.
Great observations, I already prepared a fix for the first and third one.
RightOperand
is not defined on purpose, as it can appear in many ways in JSON. If you can provide a pattern, very much appreciated!
The construct for Offer
is also on purpose, due to the fact that we need to express that target
is forbidden in most cases (everytime the schema validation goes through the Offer
path) but not always (when coming through MessageOffer
directly).
Let's use #236 to derive a solution.
The construct for
Offer
is also on purpose, due to the fact that we need to express thattarget
is forbidden in most cases (everytime the schema validation goes through the Offer path) but not always (when coming throughMessageOffer
directly).
@sebbader-sap I cannot completely follow what you are describing. But from what I understood you are referring with target
to the Agreement.odrl:target
, which is three references away. To be honest I think this is a way to complex data structure. Just imagine how somebody should implement it. You need separate classes/structs anyway, otherwise you are not able to implement a data structure, which in one case has a required attribute and in another case it is not required. A specs purpose is to be implemented, therefore it should strive for simplicity and not complexity. I would suggest to refactor this, I can make a suggestion but I need some guidance what else depends on it and needs to change accordingly.
Actually I was referring with the fourth issue to the fact, that this Json-Schema definitions
-part is a key-value map and the key in this case is #/definitions/Offer
instead of Offer
.
the key in this case is #/definitions/Offer instead of Offer.
I see, this is of course a correct finding but already merged, see https://github.com/International-Data-Spaces-Association/ids-specification/blob/36960607a67793e3fc5089655102ac6d5b9b5445/negotiation/message/schema/contract-schema.json#L105
We need to continue this discussion as soon as the Eclipse Project has been established. As the 2024-1 release in this repo is nearly done, any further corrections / extensions need to happen in the new project.
@sebbader-sap What is the status on this? I need these reported issues to be fixed or clarified. Otherwise I am partly blocked and cannot further review it..
Please also add the bug label to this issue, so that it gets better visibility..
@schoenenberg the problem is the current situation between the work inside IDSA (past) and Eclipse (future). Therefore, the general activity here is quite low. It will get better as soon as the new setup has been established.
Adding another yet one:
- [x] Context in line 58 is missing a comma
AbstractPolicyRule does not contain odrl:target:
The clause with not(required)
didn't really work out as intended, therefore, this line has been deleted.
Open topics for the Eclipse Dataspace Group:
- Schema definition of the RightOperand
- Decide on odrl:prohibition - is it needed? If so, how shall it be modelled?
@ssteinbuss please move these topics to the new repo. Apart of that, I regard the findings of this issue as solved as I also, for now, do not see a need to change the timestamp format.
[...] Apart of that, I regard the findings of this issue as solved as I also, for now, do not see a need to change the timestamp format.
@sebbader-sap I think this is rather unconventional. There are standardized timestamp formats defined like RFC3339, which is pretty similar to the Regex defined there. I compared the Regex with the definition from RFC3339 and the differences leave room for interpretation, which results in not compatible or complex implementations.
- Is there really a need for B.C. timestamps?
- 24:00:00 is not represented in the RFC3339
- Leap seconds are not represented, compare RFC3339: Section 5.6. Internet Date/Time Format and RFC3339: Appendix D. Leap Seconds
If there is no reason for it to differ from the standard, why reinventing the wheel?