Zilla is validating env vars before replacing them #797
Conversation
| @@ -190,7 +190,7 @@ private boolean validateAnnotatedSchema( | |||
| validate: | |||
| 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.
| @@ -72,6 +70,7 @@ public EngineConfigReader( | |||
| this.expressions = expressions; | |||
| this.schemaTypes = schemaTypes; | |||
| this.logger = logger; | |||
| this.annotator = new EngineConfigAnnotator(); | |||
There was a problem hiding this comment.
Please remove this assignment.
| @@ -190,7 +189,7 @@ private boolean validateAnnotatedSchema( | |||
| validate: | |||
| 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.
Description
Zilla is validating env vars before replacing them
Fixes #795