Replace the old scraping code with new WebFlux-based scraping actions #411
Conversation
mcpierce
added
the
enhancement
mcpierce
force-pushed
the
feature/issue-301
branch
from
350aab0
to
b553dd2
Compare
| * @return the list of volumes | ||
| * @throws ScrapingException if an error occurs | ||
| */ | ||
| List<ScrapingVolume> getVolumes(String apiKey, String seriesName, boolean skipCache) |
There was a problem hiding this comment.
Parameters can be declared "final" (they are final in the implementation).
There was a problem hiding this comment.
Isn’t the final keyword only useful on an implementation: ie an implementing class can drop the final keyword?
There was a problem hiding this comment.
I learn something :) I'm still a rookie ;)
It's true, final can be drop in the implementation and adding the final word in an interface can therefore be counter-intuitive as the interface contract that indicates that the parameter will not be reassigned could be reassigned in the implementation.
Don't you think then that we should remove the "final" in the scrapeComic method in the interface?
There was a problem hiding this comment.
That's what I was getting at before: "final" only applies to an actual implementation of the method and not to the interface. We could put "final" here and someone could implement the interface and leave it off and it wouldn't be a compile time error: final serves no purpose in an interface.
| * @return the issue or null | ||
| * @throws ScrapingException if an error occurs | ||
| */ | ||
| ScrapingIssue getIssue(String apiKey, Integer volume, String issueNumber, boolean skipCache) |
There was a problem hiding this comment.
Parameters can be declared "final" (they are final in the implementation).
There was a problem hiding this comment.
See previous reply.
|
|
||
| log.debug( | ||
| "Querying ComicVine for issue: id={} API key={}", this.issueId, this.getMaskedApiKey()); | ||
| final String url = this.createUrl(this.baseUrl, this.getEndpoint()); |
There was a problem hiding this comment.
if baseUrl is null couldn't that be a problem?
There was a problem hiding this comment.
Yep, thanks for catching that. I've updated the branch to include at test for that condition.
- Added dependencies on okhttp and mockwebserver - Upgraded the DbUnit dependency to 2.7.0.
This commit also provides the new classes for building additional scraping actions.
- Deleted all of the previous scraping classes. - Modified the requests to use Lombok annotations. - Added Javadoc to make the code better for new developers
mcpierce
force-pushed
the
feature/issue-301
branch
from
b553dd2
to
8cab39b
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
Status
READY
Migrations
YES
Description
Designed a new scraping system based on a ScrapingAdaptor and individual ScrapingAction objects. Replaced the old caching tables with a new concise pair of tables.