Refactor/service provider #14
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Replace Lyncode XMLIO with our forked lib - Replace any references to DSpace packages with io.gdcc
…th JDK HTTP client - Removes the old Apache Http Client dependency (good!) - Removes the old HttpOAIClient implementation based on Apache HTTP - Uses a builder pattern to create and use the OAI client - OAIClient now abstract class with a default implementation JdkHttpOaiClient - OAIClient contains an interface for the builder, also implemented inside JdkHttpOaiClient.JdkHttpBuilder - The builder interface tries to expose as much useful behaviour as possible - Lots of Javadoc - Tests for default implementation not yet implemented
In a lots of usages, the XmlReader is created for a single stream. It makes sense to use the reader within a try-with-resources block, but this requires to implement AutoClosable. Please note: the stream will not be closed by the underlying StAX parser. But as the stream is usually within the same try-with-resources block, it will be closed, too.
…ages to io.gdcc namespace
- No more log4j 1.2 (good!)
- Move stax2-api to newer version via parent POM - Update StAX2 Parser Woodstox to latest version from FasterXML - Make the parser scope runtime and optional to allow swapping for different version (appserver provided etc) or even switch to other implementation (like Aalto)
… usage - Instead of using Apache IOUtils to close the stream in a finally block, use proper try-with-resources. - Replace all lyncode/dspace packages with io.gdcc ones - Fix some vars that should be final - Fix some catch-block syntactic sugar for exceptions handled the same - Add some comments of leftover TODOs regarding deduplication etc
…eters - Also deleted from parent POM as no longer in use anywhere
…or old-style anonymous implementation
The example Item implementation method InMemoryItem.withDefaults() set the list of sets to an unmodifiable list type. Fixing with same usage of withSet() as in randomItem()
…ons to test - These are only here for testing and to provide an example. No one should use these in real applications! - As we reuse these classes from data-provider in service-provider tests, the most likely reason why these landed on the normal classpath in the first place was test classes aren't included in the data-provider JAR, which is included as testing dep in service-provider - The solution for this very simple case here is to create a tests JAR from data-provider, which can be pulled as an additional testing dependency next to the normal JAR.
Draw in the simple logging implementation for tests.
The XMLIO library is very basic and is mostly used to offer writing capabilities to xoai-common and reading capabilities to all other XOAI modules. There are classes using the writer directly due to the name conflict of io.gdcc.xoai.xml.XmlWriter and former io.gdcc.xoai.xmlio.XmlWriter. With the renaming, this cannot happen anymore. This was also annotated by Sonarcloud, and yes, it led to wrong practice. The interfaces io.gdcc.xoai.xmlio.XmlWritable and io.gdcc.xoai.xmlio.XmlReadable are not used anywhere and are duplicated with the ones with the same name in io.gdcc.xoai.xml from xoai-common module. They have been removed accordingly.
Some common classes wrongfully used the now rename XmlIoWriter class and more from the xoai-xmlio package. This is now fixed and should avoid some errors.
- Use try-with-resources, but make sure everything is closed before reading back the stream data - Throw IllegalArgumentException when the xml string could not be formatted - return an empty string is not helpful
- Use try-with-resources, but make sure everything is closed before reading back the stream data
Instead of creating just another writer, reuse the implementation already present
XmlReader.retrieveCurrentAsString was failing to add the end element of the outermost XML tag. This was noticed from failing tests in service-provider, where an XML transformer complained about the missing closing tag.
Also removing the JUnit4 and Mockito dependency, including dependencyManagement from parent POM.
Make sure the XmlReader change does not trigger any unforeseen consequences.
The module does not rely on the StAX parsing directly, as it used the xmlio and common modules to do the heavy lifting.
As Sonarcloud indicated, there are some code smells fixed here: - Make class variables final as they aren't changed again and assigned via constructor. - Refactor to use the diamond operator for generic class usages. - Avoid an NPE hazard with the strange conversion of the exception when no sets can be found.
poikilotherm
force-pushed
the
refactor/service-provider
branch
from
df27ff3 to
d924ba0
Compare
poikilotherm
force-pushed
the
refactor/service-provider
branch
from
d924ba0 to
87e09d2
Compare
|
SonarCloud Quality Gate failed. |
|
Folks, I'm shipping this now. The larger things like readbacks etc will be targeted in another iteration. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Compiles, but has failing tests.
Relates to #1
Leftover TODOs for this module:
Let service-provider parsers use an XmlReader that is closed by the handler (give reader, not stream) or create reader just when parsing and use try-with-resourcesMoved to Refactor service-provider parsers handling streams #17Add tests for new JdkHttpOaiClient implementation (target?)Moved to Add tests for JdkHttpOaiClient #18Larger TODOs for this module and beyond:
java.util.Dateandjava.util.Time