Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

AVRO-4053: doc consistency in velocity templates #3150

Merged
merged 7 commits into from
Oct 4, 2024
Merged

AVRO-4053: doc consistency in velocity templates #3150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ca41764
AVRO-4053: Improve consistency in javadoc comments
opwvhk Sep 6, 2024
3dd0954
AVRO-4053: Add test case
opwvhk Sep 6, 2024
ae42e40
AVRO-4053: rename test
opwvhk Sep 6, 2024
256976f
AVRO-4053: Fix omission
opwvhk Sep 6, 2024
b41f1eb
AVRO-4053: spotless
opwvhk Sep 6, 2024
edb0c14
AVRO-4053: Restore old name in public API
opwvhk Sep 24, 2024
e21a94c
AVRO-4053: Static imports on top
opwvhk Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 56 additions & 29 deletions lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,7 @@
*/
package org.apache.avro.compiler.specific;

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStreamWriter;
import java.io.StringWriter;
import java.io.Writer;
import java.lang.reflect.InvocationTargetException;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import static java.nio.charset.StandardCharsets.UTF_8;

import org.apache.avro.Conversion;
import org.apache.avro.Conversions;
Expand All @@ -60,7 +40,28 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static java.nio.charset.StandardCharsets.UTF_8;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStreamWriter;
import java.io.StringWriter;
import java.io.Writer;
import java.lang.reflect.InvocationTargetException;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
* Generate specific Java interfaces and classes for protocols and schemas.
Expand Down Expand Up @@ -1004,27 +1005,43 @@ public String conversionInstance(Schema schema) {
*/
public String[] javaAnnotations(JsonProperties props) {
final Object value = props.getObjectProp("javaAnnotation");
if (value == null)
return new String[0];
if (value instanceof String)
if (value instanceof String && isValidAsAnnotation((String) value))
return new String[] { value.toString() };
if (value instanceof List) {
final List<?> list = (List<?>) value;
final List<String> annots = new ArrayList<>(list.size());
for (Object o : list) {
annots.add(o.toString());
if (isValidAsAnnotation(o.toString()))
annots.add(o.toString());
}
return annots.toArray(new String[0]);
}
return new String[0];
}

private static final String PATTERN_IDENTIFIER_PART = "\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*";
private static final String PATTERN_IDENTIFIER = String.format("(?:%s(?:\\.%s)*)", PATTERN_IDENTIFIER_PART,
PATTERN_IDENTIFIER_PART);
private static final String PATTERN_STRING = "\"(?:\\\\[\\\\\"ntfb]|(?<!\\\\).)*\"";
private static final String PATTERN_NUMBER = "(?:\\((?:byte|char|short|int|long|float|double)\\))?[x0-9_.]*[fl]?";
private static final String PATTERN_LITERAL_VALUE = String.format("(?:%s|%s|true|false)", PATTERN_STRING,
PATTERN_NUMBER);
private static final String PATTERN_PARAMETER_LIST = String.format(
"\\(\\s*(?:%s|%s\\s*=\\s*%s(?:\\s*,\\s*%s\\s*=\\s*%s)*)?\\s*\\)", PATTERN_LITERAL_VALUE, PATTERN_IDENTIFIER,
PATTERN_LITERAL_VALUE, PATTERN_IDENTIFIER, PATTERN_LITERAL_VALUE);
private static final Pattern VALID_AS_ANNOTATION = Pattern
.compile(String.format("%s(?:%s)?", PATTERN_IDENTIFIER, PATTERN_PARAMETER_LIST));

private boolean isValidAsAnnotation(String value) {
martin-g marked this conversation as resolved.
Show resolved Hide resolved
return VALID_AS_ANNOTATION.matcher(value.strip()).matches();
}

// maximum size for string constants, to avoid javac limits
int maxStringChars = 8192;

/**
* Utility for template use. Takes a (potentially overly long) string and splits
* it into a quoted, comma-separted sequence of escaped strings.
* it into a quoted, comma-separated sequence of escaped strings.
*
* @param s The string to split
* @return A sequence of quoted, comma-separated, escaped strings
Expand All @@ -1036,7 +1053,7 @@ public String javaSplit(String s) throws IOException {
if (i != 0)
b.append("\",\""); // insert quote-comma-quote
String chunk = s.substring(i, Math.min(s.length(), i + maxStringChars));
b.append(javaEscape(chunk)); // escape chunks
b.append(escapeForJavaString(chunk)); // escape chunks
}
b.append("\""); // final quote
return b.toString();
Expand All @@ -1045,10 +1062,20 @@ public String javaSplit(String s) throws IOException {
/**
* Utility for template use. Escapes quotes and backslashes.
*/
public static String javaEscape(String o) {
public static String escapeForJavaString(String o) {
opwvhk marked this conversation as resolved.
Show resolved Hide resolved
return o.replace("\\", "\\\\").replace("\"", "\\\"");
}

/**
* Utility for template use (previous name). Escapes quotes and backslashes.
*
* @deprecated Use {@link #escapeForJavaString(String)} instead
*/
@Deprecated(since = "1.12.1", forRemoval = true)
public static String javaEscape(String o) {
return escapeForJavaString(o);
}

/**
* Utility for template use. Escapes comment end with HTML entities.
*/
Expand Down
4 changes: 2 additions & 2 deletions ...mpiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/enum.vm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
package $this.mangle($schema.getNamespace());
#end
#if ($schema.getDoc())
/** $schema.getDoc() */
/** $this.escapeForJavadoc($schema.getDoc()) */
#end
#foreach ($annotation in $this.javaAnnotations($schema))
@$annotation
Expand All @@ -28,7 +28,7 @@ package $this.mangle($schema.getNamespace());
public enum ${this.mangleTypeIdentifier($schema.getName())} implements org.apache.avro.generic.GenericEnumSymbol<${this.mangleTypeIdentifier($schema.getName())}> {
#foreach ($symbol in ${schema.getEnumSymbols()})${this.mangle($symbol)}#if ($foreach.hasNext), #end#end
;
public static final org.apache.avro.Schema SCHEMA$ = new org.apache.avro.Schema.Parser().parse("${this.javaEscape($schema.toString())}");
public static final org.apache.avro.Schema SCHEMA$ = new org.apache.avro.Schema.Parser().parse("${this.escapeForJavaString($schema.toString())}");
public static org.apache.avro.Schema getClassSchema() { return SCHEMA$; }

@Override
Expand Down
4 changes: 2 additions & 2 deletions ...piler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/fixed.vm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
package $this.mangle($schema.getNamespace());
#end
#if ($schema.getDoc())
/** $schema.getDoc() */
/** $this.escapeForJavadoc($schema.getDoc()) */
#end
#foreach ($annotation in $this.javaAnnotations($schema))
@$annotation
Expand All @@ -28,7 +28,7 @@ package $this.mangle($schema.getNamespace());
@org.apache.avro.specific.AvroGenerated
public class ${this.mangleTypeIdentifier($schema.getName())} extends org.apache.avro.specific.SpecificFixed {
private static final long serialVersionUID = ${this.fingerprint64($schema)}L;
public static final org.apache.avro.Schema SCHEMA$ = new org.apache.avro.Schema.Parser().parse("${this.javaEscape($schema.toString())}");
public static final org.apache.avro.Schema SCHEMA$ = new org.apache.avro.Schema.Parser().parse("${this.escapeForJavaString($schema.toString())}");
public static org.apache.avro.Schema getClassSchema() { return SCHEMA$; }
public org.apache.avro.Schema getSchema() { return SCHEMA$; }

Expand Down
8 changes: 4 additions & 4 deletions ...er/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/protocol.vm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ package $this.mangle($protocol.getNamespace());
#end

#if ($protocol.getDoc())
/** $protocol.getDoc() */
/** $this.escapeForJavadoc($protocol.getDoc()) */
#end
#foreach ($annotation in $this.javaAnnotations($protocol))
@$annotation
Expand All @@ -37,7 +37,7 @@ public interface $this.mangleTypeIdentifier($protocol.getName()) {
* $this.escapeForJavadoc($message.getDoc())
#end
#foreach ($p in $message.getRequest().getFields())##
#if ($p.doc()) * @param ${this.mangle($p.name())} $p.doc()
#if ($p.doc()) * @param ${this.mangle($p.name())} $this.escapeForJavadoc($p.doc())
#end
#end
*/
Expand All @@ -62,7 +62,7 @@ ${this.mangle($error.getFullName())}##

## Generate nested callback API
#if ($protocol.getDoc())
/** $protocol.getDoc() */
/** $this.escapeForJavadoc($protocol.getDoc()) */
#end
@org.apache.avro.specific.AvroGenerated
public interface Callback extends $this.mangleTypeIdentifier($protocol.getName()) {
Expand All @@ -78,7 +78,7 @@ ${this.mangle($error.getFullName())}##
* $this.escapeForJavadoc($message.getDoc())
#end
#foreach ($p in $message.getRequest().getFields())##
#if ($p.doc()) * @param ${this.mangle($p.name())} $p.doc()
#if ($p.doc()) * @param ${this.mangle($p.name())} $this.escapeForJavadoc($p.doc())
#end
#end
* @throws java.io.IOException The async call could not be completed.
Expand Down
32 changes: 16 additions & 16 deletions ...iler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import org.apache.avro.message.SchemaStore;
#if (${this.gettersReturnOptional} || ${this.createOptionalGetters})import java.util.Optional;#end

#if ($schema.getDoc())
/** $schema.getDoc() */
/** $this.escapeForJavadoc($schema.getDoc()) */
#end
#foreach ($annotation in $this.javaAnnotations($schema))
@$annotation
Expand Down Expand Up @@ -116,7 +116,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS

#foreach ($field in $schema.getFields())
#if ($field.doc())
/** $field.doc() */
/** $this.escapeForJavadoc($field.doc()) */
#end
#foreach ($annotation in $this.javaAnnotations($field))
@$annotation
Expand Down Expand Up @@ -155,7 +155,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS
/**
* All-args constructor.
#foreach ($field in $schema.getFields())
#if ($field.doc()) * @param ${this.mangle($field.name())} $field.doc()
#if ($field.doc()) * @param ${this.mangle($field.name())} $this.escapeForJavadoc($field.doc())
#else * @param ${this.mangle($field.name())} The new value for ${field.name()}
#end
#end
Expand Down Expand Up @@ -228,7 +228,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS
#if (${this.gettersReturnOptional} && (!${this.optionalGettersForNullableFieldsOnly} || ${field.schema().isNullable()}))
/**
* Gets the value of the '${this.mangle($field.name(), $schema.isError())}' field as an Optional&lt;${this.escapeForJavadoc(${this.javaType($field.schema())})}&gt;.
#if ($field.doc()) * $field.doc()
#if ($field.doc()) * $this.escapeForJavadoc($field.doc())
#end
* @return The value wrapped in an Optional&lt;${this.escapeForJavadoc(${this.javaType($field.schema())})}&gt;.
*/
Expand All @@ -238,7 +238,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS
#else
/**
* Gets the value of the '${this.mangle($field.name(), $schema.isError())}' field.
#if ($field.doc()) * @return $field.doc()
#if ($field.doc()) * @return $this.escapeForJavadoc($field.doc())
#else * @return The value of the '${this.mangle($field.name(), $schema.isError())}' field.
#end
*/
Expand All @@ -257,7 +257,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS
#if (${this.createOptionalGetters})
/**
* Gets the value of the '${this.mangle($field.name(), $schema.isError())}' field as an Optional&lt;${this.escapeForJavadoc(${this.javaType($field.schema())})}&gt;.
#if ($field.doc()) * $field.doc()
#if ($field.doc()) * $this.escapeForJavadoc($field.doc())
#end
* @return The value wrapped in an Optional&lt;${this.escapeForJavadoc(${this.javaType($field.schema())})}&gt;.
*/
Expand All @@ -269,7 +269,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS
#if ($this.createSetters)
/**
* Sets the value of the '${this.mangle($field.name(), $schema.isError())}' field.
#if ($field.doc()) * $field.doc()
#if ($field.doc()) * $this.escapeForJavadoc($field.doc())
#end
* @param value the value to set.
*/
Expand Down Expand Up @@ -323,7 +323,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS

#foreach ($field in $schema.getFields())
#if ($field.doc())
/** $field.doc() */
/** $this.escapeForJavadoc($field.doc()) */
#end
private ${this.javaUnbox($field.schema(), false)} ${this.mangle($field.name(), $schema.isError())};
#if (${this.hasBuilder($field.schema())})
Expand Down Expand Up @@ -402,7 +402,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS
#foreach ($field in $schema.getFields())
/**
* Gets the value of the '${this.mangle($field.name(), $schema.isError())}' field.
#if ($field.doc()) * $field.doc()
#if ($field.doc()) * $this.escapeForJavadoc($field.doc())
#end
* @return The value.
*/
Expand All @@ -413,7 +413,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS
#if (${this.createOptionalGetters})
/**
* Gets the value of the '${this.mangle($field.name(), $schema.isError())}' field as an Optional&lt;${this.escapeForJavadoc(${this.javaType($field.schema())})}&gt;.
#if ($field.doc()) * $field.doc()
#if ($field.doc()) * $this.escapeForJavadoc($field.doc())
#end
* @return The value wrapped in an Optional&lt;${this.escapeForJavadoc(${this.javaType($field.schema())})}&gt;.
*/
Expand All @@ -424,7 +424,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS

/**
* Sets the value of the '${this.mangle($field.name(), $schema.isError())}' field.
#if ($field.doc()) * $field.doc()
#if ($field.doc()) * $this.escapeForJavadoc($field.doc())
#end
* @param value The value of '${this.mangle($field.name(), $schema.isError())}'.
* @return This builder.
Expand All @@ -441,7 +441,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS

/**
* Checks whether the '${this.mangle($field.name(), $schema.isError())}' field has been set.
#if ($field.doc()) * $field.doc()
#if ($field.doc()) * $this.escapeForJavadoc($field.doc())
#end
* @return True if the '${this.mangle($field.name(), $schema.isError())}' field has been set, false otherwise.
*/
Expand All @@ -452,7 +452,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS
#if (${this.hasBuilder($field.schema())})
/**
* Gets the Builder instance for the '${this.mangle($field.name(), $schema.isError())}' field and creates one if it doesn't exist yet.
#if ($field.doc()) * $field.doc()
#if ($field.doc()) * $this.escapeForJavadoc($field.doc())
#end
* @return This builder.
*/
Expand All @@ -469,7 +469,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS

/**
* Sets the Builder instance for the '${this.mangle($field.name(), $schema.isError())}' field
#if ($field.doc()) * $field.doc()
#if ($field.doc()) * $this.escapeForJavadoc($field.doc())
#end
* @param value The builder instance that must be set.
* @return This builder.
Expand All @@ -483,7 +483,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS

/**
* Checks whether the '${this.mangle($field.name(), $schema.isError())}' field has an active Builder instance
#if ($field.doc()) * $field.doc()
#if ($field.doc()) * $this.escapeForJavadoc($field.doc())
#end
* @return True if the '${this.mangle($field.name(), $schema.isError())}' field has an active Builder instance
*/
Expand All @@ -494,7 +494,7 @@ public class ${this.mangleTypeIdentifier($schema.getName())} extends ${this.getS

/**
* Clears the value of the '${this.mangle($field.name(), $schema.isError())}' field.
#if ($field.doc()) * $field.doc()
#if ($field.doc()) * $this.escapeForJavadoc($field.doc())
#end
* @return This builder.
*/
Expand Down
Loading
Loading