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

API / Batch editing / On preview mode, return exception #7794

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fxprunayre
Copy link
Member

@fxprunayre fxprunayre commented Feb 22, 2024

When using preview mode, no report or exception are returned and it is hard to track what is wrong. Improved error reporting in preview mode for:

  • bad XPath in condition
  • bad XML snippet.

Report exception directly.

Further work would be required for xpath in target element (failing in https://github.com/geonetwork/core-geonetwork/blob/main/core/src/main/java/org/fao/geonet/kernel/EditLib.java#L945)

image

image

Checklist

  • I have read the contribution guidelines
  • Pull request provided for main branch, backports managed with label
  • Good housekeeping of code, cleaning up comments, tests, and documentation
  • Clean commit history broken into understandable chucks, avoiding big commits with hundreds of files, cautious of reformatting and whitespace changes
  • Clean commit messages, longer verbose messages are encouraged
  • API Changes are identified in commit messages
  • Testing provided for features or enhancements using automatic tests
  • User documentation provided for new features or enhancements in manual
  • Build documentation provided for development instructions in README.md files
  • Library management using pom.xml dependency management. Update build documentation with intended library use and library tutorials or documentation

Funded by Service Public de Wallonie

When using preview mode, no report or exception are returned and it is hard to track what is wrong with XPath or XML snippet.

Report exception directly.
@fxprunayre fxprunayre added this to the 4.4.3 milestone Feb 22, 2024
Copy link

Copy link
Member

@josegar74 josegar74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems invalid XPATH doesn't enter in the catch block, for example using (missing " in @xlink:href = https://metawal.wallonie.be:

/mdb:MD_Metadata/mdb:identificationInfo/*/mri:descriptiveKeywords[*/mri:thesaurusName/*/cit:title/gcx:Anchor/@xlink:href = https://metawal.wallonie.be/thesaurus/infrasig"]/*/mri:keyword/gco:CharacterString[. = 'Reporting INSPIRE']

@fxprunayre
Copy link
Member Author

it seems invalid XPATH doesn't enter in the catch block, for example using (missing " in @xlink:href = https://metawal.wallonie.be:

Indeed for the target node expression. It fails in https://github.com/geonetwork/core-geonetwork/blob/main/core/src/main/java/org/fao/geonet/kernel/EditLib.java#L945. I'm less confident changing that part where we decompose the xpath. For this PR, I propose to only concentrate on bad XML snippet and condition xpath. Maybe an option would be to try trySelectNode before going into the update - what do you think ?

@josegar74
Copy link
Member

Sounds good @fxprunayre. I'll test the items you describe that are handled

@josegar74
Copy link
Member

josegar74 commented Mar 4, 2024

@fxprunayre For me it's not really working with invalid snippet. With a service metadata (iso19115.3-2008):

  • XPath: /mdb:MD_Metadata/mdb:identificationInfo/*/srv:serviceType[1]
  • Snippet (see </co:ScopedName>):
<gco:ScopedName xmlns:gco="http://standards.iso.org/iso/19115/-3/gco/1.0" codeSpace="http://inspire.ec.europa.eu/metadata-codelist/SpatialDataServiceType">view
</co:ScopedName>

The preview doesn't fail and shows the following:

<srv:serviceType>&lt;gco:ScopedName xmlns:gco="http://standards.iso.org/iso/19115/-3/gco/1.0" codeSpace="http://inspire.ec.europa.eu/metadata-codelist/SpatialDataServiceType"&gt;view&lt;/co:ScopedName&gt;
</srv:serviceType>

@fxprunayre fxprunayre modified the milestones: 4.4.3, 4.4.4 Mar 13, 2024
@fxprunayre
Copy link
Member Author

For me it's not really working with invalid snippet. With a service metadata (iso19115.3-2008):

The snippet is not really invalid, because the value can also be text. So in your test, as it is not detected as XML, it is considered as text in https://github.com/geonetwork/core-geonetwork/blob/main/core/src/main/java/org/fao/geonet/kernel/AddElemValue.java#L44 and it is properly inserted in the XML document as text.

@fxprunayre fxprunayre modified the milestones: 4.4.4, 4.4.5 Apr 15, 2024
@fxprunayre fxprunayre modified the milestones: 4.4.5, 4.4.6 Jun 4, 2024
@fxprunayre fxprunayre modified the milestones: 4.4.6, 4.4.7 Oct 15, 2024
@CLAassistant
Copy link

CLAassistant commented Dec 8, 2024

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants