Zilla is validating env vars before replacing them #797
Conversation
| try | ||
| { | ||
| final JsonObject annotatedSchemaObject = (JsonObject) annotateJsonObject(schemaObject); | ||
| final JsonObject annotatedSchemaObject = jsonAnnotator.apply(schemaObject); |
There was a problem hiding this comment.
| final JsonObject annotatedSchemaObject = jsonAnnotator.apply(schemaObject); | |
| final EngineConfigAnnotator annotator = new EngineConfigAnnotator(); | |
| final JsonObject annotatedSchemaObject = annotator.annotate(schemaObject); |
Given the state needed by internals of annotator, suggest we just instantiate this here on stack and call the annotate method directly instead of having a field.
There was a problem hiding this comment.
That's what I was thinking but then thought that you have resaon
| this.schemaIndexes = new LinkedList<>(); | ||
| } | ||
|
|
||
| public JsonObject annotateJson( |
There was a problem hiding this comment.
| public JsonObject annotateJson( | |
| public JsonObject annotate( |
|
|
||
| import org.agrona.collections.MutableInteger; | ||
|
|
||
| public final class EngineConfigAnnotator |
There was a problem hiding this comment.
Please add EngineConfigAnnotatorTest to verify some of the important scenarios.
There was a problem hiding this comment.
Sounds good just wanted to check if that what you mean
| { | ||
| private final LinkedList<String> schemaKeys; | ||
| private final LinkedList<Integer> schemaIndexes; | ||
|
|
There was a problem hiding this comment.
Used implementation because I need an interface from both the deque and the list. Let me know if this is wrong
| private final EngineConfigAnnotator annotator; | ||
|
|
There was a problem hiding this comment.
Please remove this field.
| this.expressions = expressions; | ||
| this.schemaTypes = schemaTypes; | ||
| this.logger = logger; | ||
| this.annotator = new EngineConfigAnnotator(); |
There was a problem hiding this comment.
Please remove this assignment.
| try | ||
| { | ||
| final JsonObject annotatedSchemaObject = (JsonObject) annotateJsonObject(schemaObject); | ||
| final JsonObject annotatedSchemaObject = annotator.annotate(schemaObject); |
There was a problem hiding this comment.
| final JsonObject annotatedSchemaObject = annotator.annotate(schemaObject); | |
| final EngineConfigAnnotator() annotator = new EngineConfigAnnotator();; | |
| final JsonObject annotatedSchemaObject = annotator.annotate(schemaObject); |
This should be local because it has internal parse state, no need to carry any potential side effects of previous read to next read.
2 participants
Description
Zilla is validating env vars before replacing them
Fixes #795