Uniqueness of key for RelationshipAttributes

[britsta]

Readiness checklist

• I added/updated tests.
• I ensured that the PR title is good enough for the changelog.
• I labeled the PR.

Description

In each Relationship, there should only be at most one RelationshipAttribute for each Relationship context for a given trio of value.@type, owner and key.

Validation To Be Added

It is probably only necessary to ensure the uniqueness of the key in those cases in which new RelationshipAttributes (ThirdPartyRelationshipAttributes explicitly excluded) are created. This means, validation needs to be added for the following cases:

• ReadAttributeRequestItem with RelationshipAttributeQuery as query.
• ProposeAttributeRequestItem with RelationshipAttributeQuery as query.
• CreateAttributeRequestItem with RelationshipAttribute as attribute to be created.

To be sure, the validation is added on both sides when the Request is created and when the Request is received. Also note the following details:

• If there are several RequestItems within a Request that would lead to the creation of RelationshipAttributes with the same key, the creation of such a Request is prevented by early iteration through the RequestItems. Additionally, an error would be thrown on the side of the Recipient of the Request in the unlikely event that it has received such a Request.
• If the Recipient receives a Request that it cannot accept regardless of its specified parameters, an error is thrown. If parameters can be specified differently so that acceptance is possible, an ErrorValidationResult is returned instead.

Validation Not To Be Added

When it comes to existing RelationshipAttributes that are shared and lead to the creation of ThirdPartyRelationshipAttributes, no further validation needs to be added. There are three reasons for this:

• The uniqueness of the key is already ensured when the initial RelationshipAttributes are created.
• The re-sharing of the same RelationshipAttribute that has already been shared is already prohibited.
• It makes sense to query a ThirdPartyRelationshipAttribute with the same key again, as it can happen during the succession of RelationshipAttributes that there is another existing RelationshipAttribute with the same key that is to be read.

Open Questions

• We have to decide how to handle it when the two Identities involved in the Relationship send each other a Request to create a RelationshipAttribute with the same key and both accept it before they have received the response from their counterpart by synchronization. This can possibly be solved by forcing an early synchronization.
• Requests should not become unacceptable just because it is not possible to accept a single RequestItem by creating another RelationshipAttribute with the same key. This can perhaps be solved by adding a new ResponseItem.

Further Notes

• Note that it is a priori not possible to add validations in the applyIncomingResponseItem method of the various RequestItemProcessors that prevent the creation of a RelationshipAttribute with an already existing key, as this would lead to inconsistent data sets between the two Identities of the Relationship.
• Nested RequestItemGroups are currently not fully supported, which is why some code had to be added in a simplified version.

[codecov]

Codecov Report

Attention: Patch coverage is 91.43646% with 31 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...teAttribute/CreateAttributeRequestItemProcessor.ts	59.67%	25 Missing ⚠️
...ity/validateRelationshipAttributesWithinRequest.ts	96.29%	6 Missing ⚠️

Files with missing lines	Coverage Δ
...nsumption/src/consumption/ConsumptionCoreErrors.ts	80.69% <100.00%> (+0.20%)	⬆️
...ion/src/modules/attributes/AttributesController.ts	91.13% <100.00%> (+0.12%)	⬆️
...es/requests/incoming/IncomingRequestsController.ts	95.93% <100.00%> (+0.07%)	⬆️
...eAttribute/ProposeAttributeRequestItemProcessor.ts	91.76% <100.00%> (+1.05%)	⬆️
...readAttribute/ReadAttributeRequestItemProcessor.ts	91.74% <100.00%> (+1.02%)	⬆️
...es/requests/outgoing/OutgoingRequestsController.ts	82.31% <100.00%> (+0.20%)	⬆️
...ity/validateRelationshipAttributesWithinRequest.ts	96.29% <96.29%> (ø)
...teAttribute/CreateAttributeRequestItemProcessor.ts	70.51% <59.67%> (-25.98%)	⬇️

[britsta]

The other RequestItemProcessor tests should move to the standard of the CreateAttributeProcessor test. Also in the controller tests there coud be more options tested, although I can't tell which extent is sensible - I'd definitely want some positive tests though

I wanted to avoid too much redundancy, but will add a few more tests. :)

[britsta]

@britsta Could you add the reason for why a failed validation is sometimes an error and sometimes just a negative ValidationResult to the description or somewhere? I can't find it 😃

Hahah sure, I wanted to at least subtly differentiate whether a Request is messed up and therefore cannot be accepted regardless of the user input (this is where the concept of unacceptable Requests comes into play), or whether the user has provided inappropriate input and can accept (or create) the Request by correcting their input.

With messed up Requests I throw an error, with inappropriate user input I return a negative ValidationResult.

[Milena-Czierlinski]

I'll continue tomorrow. :)

[jkoenig134]

Blind Approval for @Milena-Czierlinski