Skip to content

Commit

Permalink
SOLR-17326: Fix references in generated SolrRequest impls (#2510)
Browse files Browse the repository at this point in the history
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 <gerlowskija@apache.org>
  • Loading branch information
malliaridis and gerlowskija committed Jun 11, 2024
1 parent a42c605 commit 461955f
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 2 deletions.
2 changes: 2 additions & 0 deletions solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ Bug Fixes

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

* SOLR-17326: Generated v2 'SolrRequest' implementations now all serialize to the correct response type (Christos Malliaridis via Jason Gerlowski)

Dependency Upgrades
---------------------
(No changes)
Expand Down
4 changes: 2 additions & 2 deletions solr/solrj/src/resources/java-template/api.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ import {{modelPackage}}.{{dataType}};
public class {{classname}} {
{{#operation}}
public static class {{operationIdCamelCase}}Response extends JacksonParsingResponse<{{returnType}}> {
public static class {{operationIdCamelCase}}Response extends JacksonParsingResponse<{{modelPackage}}.{{returnType}}> {
public {{operationIdCamelCase}}Response() {
super({{returnType}}.class);
super({{modelPackage}}.{{returnType}}.class);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.solr.client.solrj;

import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.client.solrj.request.CollectionsApi.ListCollectionsResponse;
import org.apache.solr.common.util.NamedList;
import org.junit.Assert;
import org.junit.Test;

/**
* A test ensuring that specific generated SolrRequest classes deserialize responses into the
* correct type.
*
* <p>See SOLR-17326 for more context. Consider removing once SOLR-17329 has been completed.
*/
public class ApiMustacheTemplateTests {

/**
* Tests the return type in generated response classes. This test ensures that responses are still
* returning API models in case of a naming conflict between response class and return type (API
* model).
*/
@Test
public void testParsedReturnTypes() {
JacksonParsingResponse<?> response = getJacksonParsingResponse();

Object data = null;
try {
// Parsing may fail if wrong class is used
data = response.getParsed();
} catch (Exception e) {
// Parsing should not fail for the given example.
Assert.fail("Response parsing failed with error " + e.getMessage());
}

// In case of a naming conflict, even if parsing would succeed, the type would still be the same
// as response
Assert.assertNotSame(data.getClass(), response.getClass());
Assert.assertFalse(data instanceof JacksonParsingResponse<?>);

// Currently all response types extend SolrJerseyResponse. Adjust if this change in the future.
Assert.assertTrue(data instanceof SolrJerseyResponse);
}

/**
* @return Returns a dummy response of {@link ListCollectionsResponse} with data.
*/
private static JacksonParsingResponse<?> getJacksonParsingResponse() {
JacksonParsingResponse<?> response = new ListCollectionsResponse(); // API response

// Provide a dummy response for ListCollectionsResponse
InputStream inputStream =
new ByteArrayInputStream(
"{\"responseHeader\":{\"status\":0,\"QTime\":0},\"collections\":[\"testCollection\"]}"
.getBytes(StandardCharsets.UTF_8));
NamedList<Object> responseData = new NamedList<>();
responseData.add("stream", inputStream);
response.setResponse(responseData);

return response;
}
}

0 comments on commit 461955f

Please sign in to comment.