Skip to content

Commit 1a36bd5

Browse files
SOLR-17326: Fix references in generated SolrRequest impls (#2510)
A handful of the v2 SolrRequest implementations generated by our OAS spec relied on response model classes whose names conflicted with other (unrelated) classes in solrj. This caused errors at request time as JacksonParsingResponse would try to deserialize the JSON, XML, etc. response body into these unintended classes. This commit fixes this by modifying the 'api.mustache' template so that generated SolrRequest classes now reference their response model using the fully-qualified classname (i.e. including the package). This resolves the ambiguity. --------- Co-authored-by: Jason Gerlowski <[email protected]>
1 parent 392b628 commit 1a36bd5

File tree

3 files changed

+85
-2
lines changed

3 files changed

+85
-2
lines changed

solr/CHANGES.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ Bug Fixes
6060

6161
* SOLR-17315: Fixed "next step" example urls generated by bin/solr create commands to use the newly created collection/core name. (Eric Pugh)
6262

63+
* SOLR-17326: Generated v2 'SolrRequest' implementations now all serialize to the correct response type (Christos Malliaridis via Jason Gerlowski)
64+
6365
Dependency Upgrades
6466
---------------------
6567
(No changes)

solr/solrj/src/resources/java-template/api.mustache

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,9 @@ import {{modelPackage}}.{{dataType}};
7373
public class {{classname}} {
7474
7575
{{#operation}}
76-
public static class {{operationIdCamelCase}}Response extends JacksonParsingResponse<{{returnType}}> {
76+
public static class {{operationIdCamelCase}}Response extends JacksonParsingResponse<{{modelPackage}}.{{returnType}}> {
7777
public {{operationIdCamelCase}}Response() {
78-
super({{returnType}}.class);
78+
super({{modelPackage}}.{{returnType}}.class);
7979
}
8080
}
8181

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.solr.client.solrj;
19+
20+
import java.io.ByteArrayInputStream;
21+
import java.io.InputStream;
22+
import java.nio.charset.StandardCharsets;
23+
import org.apache.solr.client.api.model.SolrJerseyResponse;
24+
import org.apache.solr.client.solrj.request.CollectionsApi.ListCollectionsResponse;
25+
import org.apache.solr.common.util.NamedList;
26+
import org.junit.Assert;
27+
import org.junit.Test;
28+
29+
/**
30+
* A test ensuring that specific generated SolrRequest classes deserialize responses into the
31+
* correct type.
32+
*
33+
* <p>See SOLR-17326 for more context. Consider removing once SOLR-17329 has been completed.
34+
*/
35+
public class ApiMustacheTemplateTests {
36+
37+
/**
38+
* Tests the return type in generated response classes. This test ensures that responses are still
39+
* returning API models in case of a naming conflict between response class and return type (API
40+
* model).
41+
*/
42+
@Test
43+
public void testParsedReturnTypes() {
44+
JacksonParsingResponse<?> response = getJacksonParsingResponse();
45+
46+
Object data = null;
47+
try {
48+
// Parsing may fail if wrong class is used
49+
data = response.getParsed();
50+
} catch (Exception e) {
51+
// Parsing should not fail for the given example.
52+
Assert.fail("Response parsing failed with error " + e.getMessage());
53+
}
54+
55+
// In case of a naming conflict, even if parsing would succeed, the type would still be the same
56+
// as response
57+
Assert.assertNotSame(data.getClass(), response.getClass());
58+
Assert.assertFalse(data instanceof JacksonParsingResponse<?>);
59+
60+
// Currently all response types extend SolrJerseyResponse. Adjust if this change in the future.
61+
Assert.assertTrue(data instanceof SolrJerseyResponse);
62+
}
63+
64+
/**
65+
* @return Returns a dummy response of {@link ListCollectionsResponse} with data.
66+
*/
67+
private static JacksonParsingResponse<?> getJacksonParsingResponse() {
68+
JacksonParsingResponse<?> response = new ListCollectionsResponse(); // API response
69+
70+
// Provide a dummy response for ListCollectionsResponse
71+
InputStream inputStream =
72+
new ByteArrayInputStream(
73+
"{\"responseHeader\":{\"status\":0,\"QTime\":0},\"collections\":[\"testCollection\"]}"
74+
.getBytes(StandardCharsets.UTF_8));
75+
NamedList<Object> responseData = new NamedList<>();
76+
responseData.add("stream", inputStream);
77+
response.setResponse(responseData);
78+
79+
return response;
80+
}
81+
}

0 commit comments

Comments
 (0)