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

Profiles with inconsistent values for previously existing Policies #1705

Closed
bmg13 opened this issue Dec 3, 2024 · 2 comments · Fixed by #1706
Closed

Profiles with inconsistent values for previously existing Policies #1705

bmg13 opened this issue Dec 3, 2024 · 2 comments · Fixed by #1706
Assignees
Labels
bug Something isn't working

Comments

@bmg13
Copy link
Contributor

bmg13 commented Dec 3, 2024

Describe the bug

When introducing profiles in Policies, the respective schema was updated accordingly. However, this did not take under consideration the fact that pre-existing policies would have a null value for this column instead of an empty collection.
So, while querying the DB for policies, a NPE is launched, breaking its use.

To Reproduce

Deploying a new instance in an infrastructure containing (stored) policies built before the addition of the profiles property. Simply retrieve (GET all, for example) policies and a 500 HTTP error will likely be seen.

Expected behavior

Backwards compatible with previous structure. Aim could be to have a default profiles value of an empty collection. Avoiding a null would be needed.

Screenshots/Error Messages

Screenshot 2024-12-03 at 22 52 35 Screenshot 2024-12-03 at 23 13 20
  • Used version: 0.8.0 of Tractus-X

Possible Implementation

An implementation with a couple of steps may solve this.
First, create an update values in table edc_policydefinitions in migrations to ensure not null existing values, similar to:

UPDATE edc_policydefinitions SET profiles='[]'::json where profiles is NULL;

Also, ensure programmatically that this condition happens, in order to avoid the NPE. A possible suggestion would be on the Upstream repo, having a validation before calling the Policy.Builder.

@bmg13 bmg13 added bug Something isn't working triage all new issues awaiting classification labels Dec 3, 2024
@ndr-brt
Copy link
Contributor

ndr-brt commented Dec 4, 2024

this was my fault, definitely, anyway curious that this has been spotted only now, the migration has been there for more than 4 months ("having release that's not deployed anywhere is like not having a release").

We could add a note in the 0.8.0 release and in the migration guide as a suggestion to workaround this problem, then provide the fix for the 0.8.1 version.

EDIT:

Also, ensure programmatically that this condition happens, in order to avoid the NPE. A possible suggestion would be on the Upstream repo, having a validation before calling the Policy.Builder.

to be honest I'm not too in favor of having this handled programmatically, because null and empty array are two different values and they need to be handled in different ways.
in this case, null is inconsistent data for this column, so the NPE is totally doing its job, and the fix must happen in the data.

@rafaelmag110
Copy link
Contributor

I guess this (as with most of other more trivial issues that should had been caught already) happens because adopters only start deploying after the version is officially released. I.e., no one keeps a deployed version of the main branch of tractusx-edc...

@rafaelmag110 rafaelmag110 removed the triage all new issues awaiting classification label Dec 4, 2024
@ndr-brt ndr-brt self-assigned this Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants