Abstraction for JSON object mapper #3319
Merged
+308
−55
Conversation
This file contains hidden or 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
…ertion implementation
…to set an implementation of a json convertion by polymorphism
…nd make it interchangeable
…esting purposes and junit tests for them
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #3319 +/- ##
============================================
- Coverage 67.03% 66.96% -0.07%
- Complexity 4656 4657 +1
============================================
Files 263 263
Lines 15061 15073 +12
Branches 949 949
============================================
- Hits 10096 10094 -2
- Misses 4560 4573 +13
- Partials 405 406 +1
... and 5 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
sazzad16
suggested changes
Mar 14, 2023
src/main/java/redis/clients/jedis/exceptions/JedisConfigException.java
Outdated
Show resolved
Hide resolved
src/main/java/redis/clients/jedis/json/parser/RedisJsonParser.java
Outdated
Show resolved
Hide resolved
src/test/java/redis/clients/jedis/modules/RedisModuleCommandsTestBase.java
Outdated
Show resolved
Hide resolved
…serializer engine configuration
sazzad16
suggested changes
Mar 14, 2023
src/test/java/redis/clients/jedis/modules/RedisModuleCommandsTestBase.java
Outdated
Show resolved
Hide resolved
src/test/java/redis/clients/jedis/modules/RedisModulesPipelineTest.java
Outdated
Show resolved
Hide resolved
src/main/java/redis/clients/jedis/json/parser/package-info.java
Outdated
Show resolved
Hide resolved
| @@ -34,6 +34,9 @@ | |||
|
|
|||
| public class CommandObjects { | |||
|
|
|||
| // initialize the parser with simple gson configuration as the default. | |||
| private Optional<JsonParser> jsonParser = Optional.of(new DefaultGsonParser()); | |||
There was a problem hiding this comment.
This invalidates #3223 where people may want to exclude Gson when they are not using RedisJSON commands.
We can do this in getJsonParser(). When it is called we can be sure that a RedisJSON command is requested and as no parser/mapper is set, we can go ahead for the default one. Note: We will need Double-checked locking.
…m word 'parser' to 'object mapper'.
…a separate package
… a jedi's Exception instead of just printing stack trace.
…ecked lock approach as suggestion in review code
…json object mapper instance in 'command object class'
sazzad16
approved these changes
Mar 15, 2023
sazzad16
changed the title
|
Merged. Thank you @clebersonp for your contribution! |
|
@sazzad16, Thanks for your time and great suggestions. 👍 😄 |
banker
added a commit
to banker/jedis
that referenced
this pull request
Mar 28, 2023
* Edited gson dependency as optional and add jackson dependency only for tests purpose * Created exception for config error purpose * Classes and interfaces created to gain code decoupling from json convertion implementation * Class is no longer needed * Interfaces created as contract for all redis command json operations to set an implementation of a json convertion by polymorphism * Changed json engine implementation to an interface to decouple code and make it interchangeable * Implemented contract to define a json engine encoding/decoding * classes created for json conversion with different json engines for testing purposes and junit tests for them * Removed import that is no longer used * Removed the optional statement for gson lib to not break the change * Just removed unused exception * Edited to use a plain jedis exception instead of a custom one * setting gson as default json parser automatically when none set * Removed unnecessary interface but kept setters for json serializer/deserializer engine configuration * Unused imports removed * Classes, interfaces, methods and other comment references renamed from word 'parser' to 'object mapper'. * Classes and interfaces moved to 'json' package because it's not need a separate package * Made some tweaks to the custom object mapper for jackson which throws a jedi's Exception instead of just printing stack trace. * Refactored method to get json object mapper reference using double checked lock approach as suggestion in review code * Removed unit test no longer occurs jedis exception after refactoring json object mapper instance in 'command object class' * Unused imports removed
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.
Framework created for redis json parser commands using abstraction for different json parser custom engines to achieve decoupling and programming code for interface instead of implementation.
Junit tests were written for the custom gson and jackson parser engine.