Skip to content

Commit fcc44d5

Browse files
committed
add empty or null check
Signed-off-by: Mingshi Liu <[email protected]>
1 parent 6b6a6de commit fcc44d5

File tree

2 files changed

+99
-8
lines changed

2 files changed

+99
-8
lines changed

server/src/main/java/org/opensearch/index/query/TemplateQueryBuilder.java

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,14 +133,16 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryCoordinatorContext) th
133133
}
134134

135135
// Convert Map<String, Object> to Map<String, String> with proper JSON escaping
136-
Map<String, String> variablesMap = contextVariables.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, entry -> {
137-
try {
138-
return JsonXContent.contentBuilder().value(entry.getValue()).toString();
139-
} catch (IOException e) {
140-
throw new RuntimeException("Error converting contextVariables to JSON string", e);
141-
}
142-
}));
143-
136+
Map<String, String> variablesMap = null;
137+
if (contextVariables != null) {
138+
variablesMap = contextVariables.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, entry -> {
139+
try {
140+
return JsonXContent.contentBuilder().value(entry.getValue()).toString();
141+
} catch (IOException e) {
142+
throw new RuntimeException("Error converting contextVariables to JSON string", e);
143+
}
144+
}));
145+
}
144146
String newQueryContent = replaceVariables(queryString, variablesMap);
145147

146148
try {
@@ -159,6 +161,16 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryCoordinatorContext) th
159161
}
160162

161163
private String replaceVariables(String template, Map<String, String> variables) {
164+
if (template == null || template.equals("null")) {
165+
throw new IllegalArgumentException("Template string cannot be null. A valid template must be provided.");
166+
}
167+
if (template.isEmpty() || template.equals("{}")) {
168+
throw new IllegalArgumentException("Template string cannot be empty. A valid template must be provided.");
169+
}
170+
if (variables == null || variables.isEmpty()) {
171+
return template;
172+
}
173+
162174
StringBuilder result = new StringBuilder();
163175
int start = 0;
164176
while (true) {

server/src/test/java/org/opensearch/index/query/TemplateQueryBuilderTests.java

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,85 @@ public void testDoRewriteWithMissingBracketVariable() throws IOException {
727727
assertTrue(exception.getMessage().contains("Unclosed variable in template"));
728728
}
729729

730+
/**
731+
* Tests the replaceVariables method when the template is null.
732+
* Verifies that an IllegalArgumentException is thrown with the appropriate error message.
733+
*/
734+
735+
public void testReplaceVariablesWithNullTemplate() {
736+
TemplateQueryBuilder templateQueryBuilder = new TemplateQueryBuilder((Map<String, Object>) null);
737+
738+
QueryCoordinatorContext queryRewriteContext = mockQueryRewriteContext();
739+
Map<String, Object> contextVariables = new HashMap<>();
740+
contextVariables.put("response", "foo");
741+
when(queryRewriteContext.getContextVariables()).thenReturn(contextVariables);
742+
743+
IllegalArgumentException exception = expectThrows(
744+
IllegalArgumentException.class,
745+
() -> templateQueryBuilder.doRewrite(queryRewriteContext)
746+
);
747+
assertEquals("Template string cannot be null. A valid template must be provided.", exception.getMessage());
748+
}
749+
750+
/**
751+
* Tests the replaceVariables method when the template is empty.
752+
* Verifies that an IllegalArgumentException is thrown with the appropriate error message.
753+
*/
754+
755+
public void testReplaceVariablesWithEmptyTemplate() {
756+
Map<String, Object> template = new HashMap<>();
757+
TemplateQueryBuilder templateQueryBuilder = new TemplateQueryBuilder(template);
758+
759+
QueryCoordinatorContext queryRewriteContext = mockQueryRewriteContext();
760+
Map<String, Object> contextVariables = new HashMap<>();
761+
contextVariables.put("response", "foo");
762+
when(queryRewriteContext.getContextVariables()).thenReturn(contextVariables);
763+
764+
IllegalArgumentException exception = expectThrows(
765+
IllegalArgumentException.class,
766+
() -> templateQueryBuilder.doRewrite(queryRewriteContext)
767+
);
768+
assertEquals("Template string cannot be empty. A valid template must be provided.", exception.getMessage());
769+
770+
}
771+
772+
/**
773+
* Tests the replaceVariables method when the variables map is null.
774+
* Verifies that the method returns the original template unchanged,
775+
* since a null variables map is treated as no replacement.
776+
*/
777+
public void testReplaceVariablesWithNullVariables() throws IOException {
778+
779+
Map<String, Object> template = new HashMap<>();
780+
Map<String, Object> term = new HashMap<>();
781+
Map<String, Object> message = new HashMap<>();
782+
783+
message.put("value", "foo");
784+
term.put("message", message);
785+
template.put("term", term);
786+
TemplateQueryBuilder templateQueryBuilder = new TemplateQueryBuilder(template);
787+
TermQueryBuilder termQueryBuilder = new TermQueryBuilder("message", "foo");
788+
789+
QueryCoordinatorContext queryRewriteContext = mockQueryRewriteContext();
790+
791+
when(queryRewriteContext.getContextVariables()).thenReturn(null);
792+
793+
TermQueryBuilder newQuery = (TermQueryBuilder) templateQueryBuilder.doRewrite(queryRewriteContext);
794+
795+
assertEquals(newQuery, termQueryBuilder);
796+
assertEquals(
797+
"{\n"
798+
+ " \"term\" : {\n"
799+
+ " \"message\" : {\n"
800+
+ " \"value\" : \"foo\",\n"
801+
+ " \"boost\" : 1.0\n"
802+
+ " }\n"
803+
+ " }\n"
804+
+ "}",
805+
newQuery.toString()
806+
);
807+
}
808+
730809
/**
731810
* Helper method to create a mock QueryCoordinatorContext for testing.
732811
*/

0 commit comments

Comments
 (0)