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

SISIS: fix regex for cover URL for Dresden Städtische Bibliotheken #599

Merged
merged 2 commits into from
Nov 5, 2020

Conversation

StefRe
Copy link
Contributor

@StefRe StefRe commented Oct 29, 2020

In Dresden, they changed the URL in the AJAX section: it now starts with "/webOPACClient/", which is also part of the opac_url.
OLD: "jsp/result..." (used for instance in Erfurt)
NEW: "/webOPACClient/jsp/result..." (used in Dresden)

So we retrieve just the part starting with "jsp/result..."

Eliminate duplicate definition of the regex in parse_search and loadDetail.

The url starts with "/webOPACClient/" here, which is also part of the opac_url,
so we retrieve just the part starting with "jsp/result..."
Eliminate duplicate definition of the regex in parse_search and loadDetail.
@johan12345
Copy link
Collaborator

The new kind "/webOPACClient/jsp/result..." seems to be a root-relative URL. Instead of solving this within the regex, I would rather suggest actually resolving the URL (so this is not dependent on the root path being called webOPACClient):

String path = matcher.group(1);
String url = new URI(opac_url + "/").resolve(path).toString();

Does this work?

to work with relative and root-relative urls.
Refactor code to avoid duplicate code.
Provide tests with real-world examples for both cases.
@StefRe
Copy link
Contributor Author

StefRe commented Nov 4, 2020

Yes, it works.
As URI methods may throw exceptions I refactored it into a separate method. This makes unit testing it much easier too.

@johan12345
Copy link
Collaborator

That looks good, thank you!

@johan12345 johan12345 merged commit a98b8d7 into opacapp:master Nov 5, 2020
@StefRe StefRe deleted the fix/SISIS_cover_pattern branch November 5, 2020 20:12
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.

None yet

2 participants