Skip to content

Dra 1788 load solrshield for each collection#73

Open
davidgrove73 wants to merge 16 commits intomasterfrom
DRA-1788-load-solrshield-for-each-collection
Open

Dra 1788 load solrshield for each collection#73
davidgrove73 wants to merge 16 commits intomasterfrom
DRA-1788-load-solrshield-for-each-collection

Conversation

@davidgrove73
Copy link
Copy Markdown
Contributor

@davidgrove73 davidgrove73 commented Mar 31, 2026

Enable SolrShield to be loaded per collection with seperate configurations.
The following has been implemented:

  • Decoupling the SolrShield config from the rest of the ds-discover config
  • Refactored SolrShield from static utility class to instance class
  • Add logic to per collection instaciation of SolrShield in SolrManager
  • Removed the global extraAllowedParameters (not quite sure about this).

@davidgrove73
Add comprehensive SolrShield tests before refactoring
c3cef05
@davidgrove73
Refactor SolrShield from static utility to instance class
4a8954d 
Static delegate methods preserved for backwards compatibility.
@davidgrove73
Move extraAllowedParameters into shield config as passthroughParameters
bcefbe2 
Decouples Profile from ServiceConfig. The passthrough list is now part
of the shield YAML, read by Profile directly.
@davidgrove73
Add per-collection shield path storage and lazy getShield() to SolrMa…
c1c83cd 
…nager
@davidgrove73
Switch DsDiscoverApiServiceImpl to per-collection shield via SolrManager
0fe0761 
Remove static SolrShield.ensureConfig() from ContextListener. Add
per-collection shield tests with permissive and restrictive configs.
@davidgrove73
Extract shield config to standalone file, add shield path to collection
24ae497 
New standalone ds-solrshield.yaml replaces the embedded solr.shield
section. Collection config now references it via the shield key.
@davidgrove73
refactored tests
f4cc6e6
@davidgrove73
DRA-1788
eddf15b 
Add defType to params in SearchComponent.java
@davidgrove73
DRA-1788
9c6d598 
Enable load of solrshield
@davidgrove73
DRA-1788
73ae135 
Remove global extraAllowParameters and the global filter logic, and replace with a per collection filter.
seskildsen
seskildsen requested changes Apr 7, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@seskildsen seskildsen left a comment

Choose a reason for hiding this comment

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

Some changes.

Comment thread src/main/java/dk/kb/discover/util/solrshield/SolrShield.java
Comment thread src/main/java/dk/kb/discover/util/solrshield/SolrShield.java Outdated
Comment thread src/main/java/dk/kb/discover/util/solrshield/SolrShield.java Outdated
Comment thread src/test/java/dk/kb/discover/util/solrshield/SolrShieldTest.java
Comment thread src/test/java/dk/kb/discover/util/solrshield/SolrShieldTest.java
Comment thread src/test/java/dk/kb/discover/util/solrshield/SolrShieldTest.java Outdated
Comment thread src/test/java/dk/kb/discover/util/solrshield/SolrShieldTest.java Outdated
Comment thread src/test/java/dk/kb/discover/util/solrshield/SolrShieldTest.java Outdated
Comment thread src/main/java/dk/kb/discover/api/v1/impl/DsDiscoverApiServiceImpl.java Outdated
Comment thread src/main/java/dk/kb/discover/api/v1/impl/DsDiscoverApiServiceImpl.java Outdated
@thomasegense
Copy link
Copy Markdown
Contributor

thomasegense commented Apr 9, 2026

When solrshield blocks a call only log a WARN with the message, and not whole stacktrace.

@thomasegense
Copy link
Copy Markdown
Contributor

thomasegense commented Apr 9, 2026

It have deployed on ds-devel01 and testet it is working! Very nice work.

thomasegense
thomasegense requested changes Apr 9, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@thomasegense thomasegense left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Two changes:

  1. Double logic in serviceAPI class, would also like to move it to facade class.
  2. Do not log full stacktrace only single line WARN with message, if solrshield blocks request.

Comment thread src/main/java/dk/kb/discover/api/v1/impl/DsDiscoverApiServiceImpl.java Outdated
@davidgrove73
DRA-1788
f28d9e8 
Remove '<p>' from javadoc
@davidgrove73
DRA-1788
2db7281 
cosmetic changes from review
@davidgrove73
DRA-1788
0e5be8a 
Do print exception stack trace on service exception thronw by solrshield. Bypass handleException.
@davidgrove73
DRA-1788
494cf3c 
Refactor: extract shield evaluation to own method
thomasegense
thomasegense approved these changes Apr 14, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@thomasegense thomasegense left a comment

Choose a reason for hiding this comment

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

LGTM

@davidgrove73
DRA-1788
33c4e46 
Removed extra newlines
@davidgrove73
DRA-1788
4879f3a 
cosmetic changes
seskildsen
seskildsen approved these changes Apr 15, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@seskildsen seskildsen left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looking good. Remember to squash and merge. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thomasegense thomasegense thomasegense approved these changes

@seskildsen seskildsen seskildsen approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@davidgrove73 @thomasegense @seskildsen