dsmiley commented on code in PR #1793:
URL: https://github.com/apache/solr/pull/1793#discussion_r1270167635
##########
solr/solrj-model/src/test/org/apache/solr/model/DummyTest.java:
##########
@@ -0,0 +1,11 @@
+package org.apache.solr.model;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.junit.Test;
+
+public class DummyTest extends SolrTestCaseJ4 {
Review Comment:
Why was this added?
##########
solr/solrj/build.gradle:
##########
@@ -15,13 +15,82 @@
* limitations under the License.
*/
+plugins {
+ id "org.openapi.generator" version "6.6.0"
+}
apply plugin: 'java-library'
+apply plugin: 'com.diffplug.spotless'
+
+import com.diffplug.gradle.spotless.JavaExtension
+
description = 'Solrj - Solr Java Client'
+configurations {
+ openApiSpecFile {
+ canBeConsumed = false
+ }
+ generatedCode
+}
+
+tasks.register("copySpec", Copy) {
+ from("../solrj-model/build/generated/openapi")
+ into(layout.buildDirectory.dir("openapispec"))
+}
+
+def generatedExt = new JavaExtension(spotless)
+project.ext.spotlessJavaSetup.execute(generatedExt)
+generatedExt.target("build/generated/src/**/*.java")
+def generatedSpotlessTask =
generatedExt.createIndependentApplyTask("generatedSpotless")
+generatedSpotlessTask.group("build")
+generatedSpotlessTask.description("Apply formatting for generated code")
+
+openApiGenerate {
+ generatorName = "java"
+ // TODO - pretty hacky to specify inputSpec this way...ideally I'd just rely
on a property defined in
+ // solrj-model, as below, but it doesn't appear to be working
+ //inputSpec = project(":solr:solrj-model").ext.openapispecfile
+ inputSpec = "${buildDir}/openapispec/openapi.json"
+
+ // Add 'debugModels: ""' or 'debugOperations: ""' to get the JSON input to
mustache templating for those components
+ globalProperties.set([modelDocs: "false", apis: "", models: ""])
+ templateDir = "$projectDir/src/resources/java-template"
+ apiPackage = "org.apache.solr.client.solrj.request"
+ modelPackage = "org.apache.solr.client.solrj.model"
+ outputDir = "${buildDir}/generated/"
+ generateApiTests = false
+ generateModelTests = false
+}
+
+tasks.getByName("copySpec").dependsOn
configurations.getByName("openApiSpecFile")
Review Comment:
Why do this in a dynamic/reflection-like way; why not explicitly do this at
the point that copySpec is declared?
##########
solr/solrj/src/java/org/apache/solr/common/util/ModelConstants.java:
##########
@@ -0,0 +1,30 @@
+/*
+ * 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.common.util;
+
+// TODO This probably shouldn't exist in a final product, but it's fine for a
POC
Review Comment:
Out practice has been the magic string "nocommit" which we prevent merging
but I see you use "TODO"... could go unnoticed as we don't have tooling to
point these out. I use "TODO" in a loose way but here you meant nocommit I
think
##########
solr/solrj/build.gradle:
##########
@@ -15,13 +15,82 @@
* limitations under the License.
*/
+plugins {
+ id "org.openapi.generator" version "6.6.0"
+}
apply plugin: 'java-library'
+apply plugin: 'com.diffplug.spotless'
+
+import com.diffplug.gradle.spotless.JavaExtension
+
description = 'Solrj - Solr Java Client'
+configurations {
+ openApiSpecFile {
+ canBeConsumed = false
+ }
+ generatedCode
+}
+
+tasks.register("copySpec", Copy) {
+ from("../solrj-model/build/generated/openapi")
+ into(layout.buildDirectory.dir("openapispec"))
+}
+
+def generatedExt = new JavaExtension(spotless)
+project.ext.spotlessJavaSetup.execute(generatedExt)
+generatedExt.target("build/generated/src/**/*.java")
+def generatedSpotlessTask =
generatedExt.createIndependentApplyTask("generatedSpotless")
+generatedSpotlessTask.group("build")
+generatedSpotlessTask.description("Apply formatting for generated code")
+
+openApiGenerate {
+ generatorName = "java"
+ // TODO - pretty hacky to specify inputSpec this way...ideally I'd just rely
on a property defined in
+ // solrj-model, as below, but it doesn't appear to be working
+ //inputSpec = project(":solr:solrj-model").ext.openapispecfile
+ inputSpec = "${buildDir}/openapispec/openapi.json"
+
+ // Add 'debugModels: ""' or 'debugOperations: ""' to get the JSON input to
mustache templating for those components
+ globalProperties.set([modelDocs: "false", apis: "", models: ""])
+ templateDir = "$projectDir/src/resources/java-template"
+ apiPackage = "org.apache.solr.client.solrj.request"
+ modelPackage = "org.apache.solr.client.solrj.model"
+ outputDir = "${buildDir}/generated/"
+ generateApiTests = false
+ generateModelTests = false
+}
+
+tasks.getByName("copySpec").dependsOn
configurations.getByName("openApiSpecFile")
+tasks.getByName("openApiGenerate").dependsOn tasks.copySpec
+tasks.getByName("openApiGenerate").finalizedBy generatedSpotlessTask
+generatedSpotlessTask.dependsOn tasks.openApiGenerate
+
+sourceSets {
+ main {
+ java {
+ srcDir "${buildDir}/generated/src/main/java"
+ }
+ }
+}
+tasks.compileJava.dependsOn(configurations.generatedCode)
+
+
dependencies {
+ openApiSpecFile(project(path: ":solr:solrj-model", configuration:
"openapiSpec"))
+ generatedCode files("${buildDir}/generated/main/java") {
+ builtBy tasks.openApiGenerate
+ }
+
+ // TODO Most likely temporary, due to historical pushback on adding Jackson
deps to solrj, but it does _hugely_
+ // simplify the parsing code and there are new compelling reasons, given
the claims around CBOR performance relative
+ // to JSON, javabin, etc.
+ implementation 'com.fasterxml.jackson.core:jackson-databind' // Came from a
testImplementation line below.
Review Comment:
I was certainly pushing back in years past but I can see there is a huge
code maintenance win here so I'm in favor.
##########
solr/solrj/src/resources/java-template/pojo.mustache:
##########
@@ -0,0 +1,391 @@
+public class {{classname}} {
+
+ {{#vars}}
+ public static final String JSON_PROPERTY_{{nameInSnakeCase}} =
"{{baseName}}";
+ {{#isContainer}}
+ private {{{datatypeWithEnum}}} {{name}}{{#defaultValue}} =
{{{.}}}{{/defaultValue}};
+ {{/isContainer}}
+ {{^isContainer}}
+
{{#isDiscriminator}}protected{{/isDiscriminator}}{{^isDiscriminator}}private{{/isDiscriminator}}
{{{datatypeWithEnum}}} {{name}}{{#defaultValue}} = {{{.}}}{{/defaultValue}};
+ {{/isContainer}}
+
+ {{/vars}}
+ public {{classname}}() {}
+
+ {{#vars}}
+ {{^isReadOnly}}
+ public {{classname}} {{name}}({{{datatypeWithEnum}}} {{name}}) {
+ this.{{name}} = {{name}};
+ return this;
+ }
+
+ {{/isReadOnly}}
+ /**
+ {{#description}}
+ * {{.}}
+ {{/description}}
+ {{^description}}
+ * Get {{name}}
+ {{/description}}
+ {{#minimum}}
+ * minimum: {{.}}
+ {{/minimum}}
+ {{#maximum}}
+ * maximum: {{.}}
+ {{/maximum}}
+ * @return {{name}}
+ {{#deprecated}}
+ * @deprecated
+ {{/deprecated}}
+ */
+{{#deprecated}}
+ @Deprecated
+{{/deprecated}}
+{{> jackson_annotations}} public {{{datatypeWithEnum}}} {{getter}}() {
+ return {{name}};
+ }
+
+ {{^isReadOnly}}
+{{> jackson_annotations}} public void {{setter}}({{{datatypeWithEnum}}}
{{name}}) {
+ this.{{name}} = {{name}};
+ }
+ {{/isReadOnly}}
+
+ {{/vars}}
+ {{#parent}}
+ {{#allVars}}
+ {{#isOverridden}}
+ @Override
+ public {{classname}} {{name}}({{{datatypeWithEnum}}} {{name}}) {
+ {{#vendorExtensions.x-is-jackson-optional-nullable}}
+ this.{{setter}}(JsonNullable.<{{{datatypeWithEnum}}}>of({{name}}));
+ {{/vendorExtensions.x-is-jackson-optional-nullable}}
+ {{^vendorExtensions.x-is-jackson-optional-nullable}}
+ this.{{setter}}({{name}});
+ {{/vendorExtensions.x-is-jackson-optional-nullable}}
+ return this;
+ }
+
+ {{/isOverridden}}
+ {{/allVars}}
+ {{/parent}}
+ @Override
+ public boolean equals(Object o) {
+ {{#useReflectionEqualsHashCode}}
+ return EqualsBuilder.reflectionEquals(this, o, false, null, true);
+ {{/useReflectionEqualsHashCode}}
+ {{^useReflectionEqualsHashCode}}
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }{{#hasVars}}
+ {{classname}} {{classVarName}} = ({{classname}}) o;
+ return
{{#vars}}{{#vendorExtensions.x-is-jackson-optional-nullable}}equalsNullable(this.{{name}},
{{classVarName}}.{{name}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{#isByteArray}}Arrays{{/isByteArray}}{{^isByteArray}}Objects{{/isByteArray}}.equals(this.{{name}},
{{classVarName}}.{{name}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{^-last}}
&&
+ {{/-last}}{{/vars}}{{#parent}} &&
+ super.equals(o){{/parent}};{{/hasVars}}{{^hasVars}}
+ return
{{#parent}}super.equals(o){{/parent}}{{^parent}}true{{/parent}};{{/hasVars}}
+ {{/useReflectionEqualsHashCode}}
+ }{{#vendorExtensions.x-jackson-optional-nullable-helpers}}
+
+ private static <T> boolean equalsNullable(JsonNullable<T> a, JsonNullable<T>
b) {
+ return a == b || (a != null && b != null && a.isPresent() && b.isPresent()
&& Objects.deepEquals(a.get(), b.get()));
+ }{{/vendorExtensions.x-jackson-optional-nullable-helpers}}
+
+ @Override
+ public int hashCode() {
+ {{#useReflectionEqualsHashCode}}
+ return HashCodeBuilder.reflectionHashCode(this);
+ {{/useReflectionEqualsHashCode}}
+ {{^useReflectionEqualsHashCode}}
+ return
Objects.hash({{#vars}}{{#vendorExtensions.x-is-jackson-optional-nullable}}hashCodeNullable({{name}}){{/vendorExtensions.x-is-jackson-optional-nullable}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{^isByteArray}}{{name}}{{/isByteArray}}{{#isByteArray}}Arrays.hashCode({{name}}){{/isByteArray}}{{/vendorExtensions.x-is-jackson-optional-nullable}}{{^-last}},
{{/-last}}{{/vars}}{{#parent}}{{#hasVars}},
{{/hasVars}}super.hashCode(){{/parent}});
+ {{/useReflectionEqualsHashCode}}
+ }{{#vendorExtensions.x-jackson-optional-nullable-helpers}}
+
+ private static <T> int hashCodeNullable(JsonNullable<T> a) {
+ if (a == null) {
+ return 1;
+ }
+ return a.isPresent() ? Arrays.deepHashCode(new Object[]{a.get()}) : 31;
+ }{{/vendorExtensions.x-jackson-optional-nullable-helpers}}
+
+ @Override
+ public String toString() {
+ StringBuilder sb = new StringBuilder();
+ sb.append("class {{classname}} {\n");
+ {{#parent}}
+ sb.append(" ").append(toIndentedString(super.toString())).append("\n");
+ {{/parent}}
+ {{#vars}}
+ sb.append(" {{name}}:
").append(toIndentedString({{name}})).append("\n");
+ {{/vars}}
+ sb.append("}");
+ return sb.toString();
+ }
+
+ /**
+ * Convert the given object to string with each line indented by 4 spaces
+ * (except the first line).
+ */
+ private{{#jsonb}} static{{/jsonb}} String toIndentedString(Object o) {
+ if (o == null) {
+ return "null";
+ }
+ return o.toString().replace("\n", "\n ");
+ }
+{{#supportUrlQuery}}
+
+ /**
+ * Convert the instance into URL query string.
+ *
+ * @return URL query string
+ */
+ public String toUrlQueryString() {
+ return toUrlQueryString(null);
+ }
+
+ /**
+ * Convert the instance into URL query string.
+ *
+ * @param prefix prefix of the query string
+ * @return URL query string
+ */
+ public String toUrlQueryString(String prefix) {
+ String suffix = "";
+ String containerSuffix = "";
+ String containerPrefix = "";
+ if (prefix == null) {
+ // style=form, explode=true, e.g. /pet?name=cat&type=manx
+ prefix = "";
+ } else {
+ // deepObject style e.g. /pet?id[name]=cat&id[type]=manx
+ prefix = prefix + "[";
+ suffix = "]";
+ containerSuffix = "]";
+ containerPrefix = "[";
+ }
+
+ StringJoiner joiner = new StringJoiner("&");
+
+ {{#allVars}}
+ // add `{{baseName}}` to the URL query string
+ {{#isArray}}
+ {{#items.isPrimitiveType}}
+ {{#uniqueItems}}
+ if ({{getter}}() != null) {
+ int i = 0;
+ for ({{{items.dataType}}} _item : {{getter}}()) {
+ try {
+ joiner.add(String.format("%s{{baseName}}%s%s=%s", prefix, suffix,
+ "".equals(suffix) ? "" : String.format("%s%d%s",
containerPrefix, i, containerSuffix),
+ URLEncoder.encode(String.valueOf(_item),
"UTF-8").replaceAll("\\+", "%20")));
+ } catch (UnsupportedEncodingException e) {
+ // Should never happen, UTF-8 is always supported
+ throw new RuntimeException(e);
+ }
+ }
+ i++;
+ }
+ {{/uniqueItems}}
+ {{^uniqueItems}}
+ if ({{getter}}() != null) {
+ for (int i = 0; i < {{getter}}().size(); i++) {
+ try {
+ joiner.add(String.format("%s{{baseName}}%s%s=%s", prefix, suffix,
+ "".equals(suffix) ? "" : String.format("%s%d%s",
containerPrefix, i, containerSuffix),
+ URLEncoder.encode(String.valueOf({{getter}}().get(i)),
"UTF-8").replaceAll("\\+", "%20")));
+ } catch (UnsupportedEncodingException e) {
+ // Should never happen, UTF-8 is always supported
+ throw new RuntimeException(e);
+ }
+ }
+ }
+ {{/uniqueItems}}
+ {{/items.isPrimitiveType}}
+ {{^items.isPrimitiveType}}
+ {{#items.isModel}}
+ {{#uniqueItems}}
+ if ({{getter}}() != null) {
+ int i = 0;
+ for ({{{items.dataType}}} _item : {{getter}}()) {
+ if (_item != null) {
+
joiner.add(_item.toUrlQueryString(String.format("%s{{baseName}}%s%s", prefix,
suffix,
+ "".equals(suffix) ? "" : String.format("%s%d%s",
containerPrefix, i, containerSuffix))));
+ }
+ }
+ i++;
+ }
+ {{/uniqueItems}}
+ {{^uniqueItems}}
+ if ({{getter}}() != null) {
+ for (int i = 0; i < {{getter}}().size(); i++) {
+ if ({{getter}}().get(i) != null) {
+
joiner.add({{getter}}().get(i).toUrlQueryString(String.format("%s{{baseName}}%s%s",
prefix, suffix,
+ "".equals(suffix) ? "" : String.format("%s%d%s",
containerPrefix, i, containerSuffix))));
+ }
+ }
+ }
+ {{/uniqueItems}}
+ {{/items.isModel}}
+ {{^items.isModel}}
+ {{#uniqueItems}}
+ if ({{getter}}() != null) {
+ int i = 0;
+ for ({{{items.dataType}}} _item : {{getter}}()) {
+ if (_item != null) {
+ try {
+ joiner.add(String.format("%s{{baseName}}%s%s=%s", prefix, suffix,
+ "".equals(suffix) ? "" : String.format("%s%d%s",
containerPrefix, i, containerSuffix),
+ URLEncoder.encode(String.valueOf(_item),
"UTF-8").replaceAll("\\+", "%20")));
+ } catch (UnsupportedEncodingException e) {
+ // Should never happen, UTF-8 is always supported
+ throw new RuntimeException(e);
+ }
+ }
+ i++;
+ }
+ }
+ {{/uniqueItems}}
+ {{^uniqueItems}}
+ if ({{getter}}() != null) {
+ for (int i = 0; i < {{getter}}().size(); i++) {
+ if ({{getter}}().get(i) != null) {
+ try {
+ joiner.add(String.format("%s{{baseName}}%s%s=%s", prefix, suffix,
Review Comment:
Remember ROOT locale for all these formats
##########
solr/solrj/src/java/org/apache/solr/client/solrj/JacksonContentWriter.java:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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 com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.IOException;
+import java.io.OutputStream;
+import org.apache.solr.client.solrj.request.RequestWriter;
+
+public class JacksonContentWriter implements RequestWriter.ContentWriter {
+
+ private final Object toWrite;
+ private final String contentType;
+
+ public JacksonContentWriter(String contentType, Object toWrite) {
+ this.contentType = contentType;
+ this.toWrite = toWrite;
+ }
+
+ @Override
+ public void write(OutputStream os) throws IOException {
+ new ObjectMapper().writeValue(os, toWrite);
Review Comment:
share the ObjectMapper?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
