JSON Array splitting #77
Open
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
51-code
added
assistance
51-code
commented
Oct 1, 2024
…tests for splitType
51-code
added
review
and removed
assistance
eemhu
reviewed
Oct 2, 2024
| IMessageHandler conversion = new RelpConversion(pool, securityConfig, basicAuthentication, lookupConfig); | ||
|
|
||
| // apply splitting if configured | ||
| switch (payloadConfig.splitType) { // payloadConfig is Validateable, no need to check wrong config here |
There was a problem hiding this comment.
could switch-case be replaced with another object?
There was a problem hiding this comment.
Tried to refactor code for creating a decorator for this in NettyHttpServer and even some other objects that use / forward the RelpConversion object. This was so that I could put the config checking code there so it doesn't clutter the other code. This however seems to be impossible now with objects extending and implementing 3rd party objects that are not immutable. Would need a complete refactoring really.
Would it be terrible to create an object just for creating this Conversion object with the config? Not really OOP style, but the code would be somewhere else than in the Main class.
eemhu
reviewed
Oct 2, 2024
| splitRegex = propertiesReader.getStringProperty("payload.splitRegex"); | ||
| splitEnabled = propertiesReader.getBooleanProperty("payload.splitEnabled"); | ||
| this.regexPattern = propertiesReader.getStringProperty("payload.splitType.regex.pattern"); | ||
| this.splitType = propertiesReader.getStringProperty("payload.splitType"); |
eemhu
reviewed
Oct 2, 2024
src/main/java/com/teragrep/lsh_01/conversion/DefaultPayload.java
Outdated
Show resolved
Hide resolved
eemhu
reviewed
Oct 2, 2024
src/main/java/com/teragrep/lsh_01/conversion/RegexConversion.java
Outdated
Show resolved
Hide resolved
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.
Adds a new splitting option: json_array. Takes HTTP requests with this kind of format:
and converts each of the JSON objects to their own syslog message and sends them forward.
Configuration had changes:
payload.splitEnabledwas changed topayload.splitType. Valid values are regex, json_array or none. none is the default.payload.splitRegexwas moved topayload.splitType.regex.pattern.Regex splitting was already supported, but the design was not scalable. Therefore it was refactored as well to allow json_array to be implemented.
There was a problem with Jakarta's way of getting a JSON object as a string. It removes any whitespace from the String representation. This is okay as the content stays the same.
Tests most importantly include:
JsonPayloadTestfor unit tests for the new functionalityJsonSplittingTestfor end-to-end testing the functionality with actual HTTP-requests sent to the component