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

Treat eId as mandatory value #593

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SebastianRossa
Copy link
Contributor

@SebastianRossa SebastianRossa commented Sep 11, 2024

In the context of introducing the custom standardized error responses in the backend we had to do some refactoring so that all exceptions are handled and converted to the ProblemDetail object.

We realized that we are using in many places the default NoSuchElementException, which is thrown, when on a Optional the orElseThrow() is used. We wanted to replace this default with a custom one, like maybe using the MandatoryNodeNotFoundException. But we realized that many of this default usage comes when trying to get the eId of an element.

We started therefore reformating the fromNode function of the Eid class so that not an optional is returned, since we know all/many elements MUST have an eId. We could then just filter out those elements passed to that function that do not belong to the list of nodes with mandatory eId.

But we realized that firstly with LDML.de 1.7 almost every XML node will get a mandatory eId (like for example akn:meta and many children there). Therefore we park this implementation for when:

  1. XSD validation after upload is in place, because we can then be sure that our DB won't have amending laws with elements without eId attribute (if they must have it)
  2. We convert to LDML.de 1.7 because almost every node will need to have an eId.

We added for now an exception handler for the NoSuchElementException as a fallback till we go on with this work.

@SebastianRossa SebastianRossa added the do not merge For sharing prototypes or ideas that are not intended for merging label Sep 11, 2024
Copy link

sonarcloud bot commented Sep 11, 2024

Copy link

sonarcloud bot commented Sep 11, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
1 New Code Smells (required ≤ 0)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge For sharing prototypes or ideas that are not intended for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant