search

openBackhaul / NotificationProxy

Consolidates notifications into specific streams
Apache License 2.0
1 stars 1 forks source link

ControlStruct-Validation #31

Closed FelixFleckensteinMentopolis closed 9 months ago

FelixFleckensteinMentopolis commented 10 months ago

While testing the various calls in the NotificationProxy API we discoverd a problem calling the getControlConstruct Core API. There is an issue validating the „static-value“ and „value-reference“ attributes of the „response-profile-configuration“ structure. The OpenAPI file specifies the following for response-profile-configuration: maxProperties: 1 minProperties: 1

This leads to validation errors when querying the control struct from the NotificationProxy. It seems that there should be a XOR requirement for static-value and value-reference (only one of them is required at the same time). However, the specification is treaded as if both are attributes are mandatory. As a work around, we inserted dummy values for static-value in the load.json file.

openBackhaul commented 10 months ago

I can't reproduce the problem from the specification.

If both attributes would be mandatory, there would be a required: statement that is listing both attributes. Such required: statement does not exist in any of the 4 occurrences.

The combination of (minProperties: 1 + maxProperties: 1 + additionalProperties: false) is expected to work like an XOR.

The NP CONFIGfile does not contain any instance of static-value attribute, but a value-reference attribute in all instances of response-profile-configuration. So, it unclear where a conflict with (minProperties: 1 + maxProperties: 1 + additionalProperties: false) could come from.

In principle, it would be feasible to replace the existing structure by a oneOf: statements that describes two different response-profile-configuration objects with either static-value or value-reference attribute. This would be a change to the ApplicationPattern and effect all applications. Before going this direction, I would like to know why the problem did not occur before.

MFMentopolis commented 9 months ago

This might be fixed by this commit for v1.0.1_spec: https://github.com/openBackhaul/NotificationProxy/commit/e955de10dd12f47b0c72768152f7a848d9ce03fa We will check this if the new spec is available.

openBackhaul commented 9 months ago

The additionalProperties: false statements will be added to the action-profile-1-0:action-profile-pac as a part of the ApplicationPattern 2.1.0. AP 2.1.0 is currently under implementation. The NotificationProxy needs to be updated to the AP 2.1.0 as a part of its v1.1.0_spec.

If you could easily do, please add the two additionalProperties: false statements as described in e955de1 as a preliminary solution.

(I am closing this issue, because it will be automatically solved by forthcoming update to AP 2.1.0.)