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

Formatter / Withheld element not always hidden. #7641

Merged
merged 1 commit into from
Jan 23, 2024

Conversation

fxprunayre
Copy link
Member

@fxprunayre fxprunayre commented Jan 18, 2024

Affected API calls:

For testing, add a record with 2 online resources. Add withheld attribute to one of them, call the related API. Sometimes 1 or 2 links are returned (it does not happen always, but usually if accessing the same API call with a browser and Curl using the same session the problem can be observed). A variable was set using ThreadLocal which looks to be the cause of this. It was noticed while opening the editor not always listing all distribution links. Withheld elements are always hidden properly for anonymous user.

for i in {1..20}
do
 curl -s 'http://localhost:8080/geonetwork/srv/api/records/720ceda7-a9ea-4bbe-896a-04925c44a7f0/related?type=thumbnails&type=onlines' \
  -H "cookie: XSRF-TOKEN=4c2a7847-87f6-4cf6-bb55-173e43332729; JSESSIONID=$JSESSIONID.node0; " \
  -H 'x-xsrf-token: 4c2a7847-87f6-4cf6-bb55-173e43332729' | grep -o -i "<item>" | wc -l
done
1
2
1
2
1
1

The Formatter API has a hide_withheld parameter to hide with held elements even if the user is allowed to see them. The application is not using this parameter, and opening an incognito window can be used to check if with held elements are properly removed.

This change propose to remove the ThreadLocal variable and only rely on user session and edits rights of the current user to hide or not with held elements.

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

Affected API calls:
* http://localhost:8080/geonetwork/srv/api/records/720ceda7-a9ea-4bbe-896a-04925c44a7f0/formatters/xyz
* http://localhost:8080/geonetwork/srv/api/records/720ceda7-a9ea-4bbe-896a-04925c44a7f0/related?type=thumbnails&type=onlines

For testing, add a record with 2 online resources. Add withheld attribute to one of them, call the related API.
Sometimes 1 or 2 links are returned (it does not happen always, but usually if accessing the same API call with a browser and Curl using the same session the problem can be observed). A variable was set using ThreadLocal which looks to be the cause of this. It was noticed while opening the editor not always listing all distribution links.

The Formatter API has a `hide_withheld` parameter to hide with held elements even if the user is allowed to see them. The application is not using this parameter, and opening an incognito window can be used to check if with held elements are properly removed.

This change propose to remove the ThreadLocal variable and only rely on user session and edits rights of the current user to hide or not with held elements.
@fxprunayre fxprunayre added the api change Indicate a change in the API label Jan 18, 2024
@fxprunayre fxprunayre added this to the 4.4.3 milestone Jan 18, 2024
@fxprunayre fxprunayre requested a review from josegar74 January 18, 2024 14:28
Copy link

sonarcloud bot commented Jan 18, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

2 New issues
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

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.

I have tested to run the script in parallel in different terminals, but even without the changes, the results are always fine.

In any case, the change looks fine. Apparently the code was done to offer administrators the option to visualise how would look the metadata for users that are not allowed to display the elements with withheld, but as you indicate there is an alternative solution, and the code removed is quite obscure.

@fxprunayre fxprunayre merged commit a65a995 into main Jan 23, 2024
10 checks passed
@fxprunayre fxprunayre deleted the 44-withheldelementsnotalwaysfiltered branch January 23, 2024 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api change Indicate a change in the API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants