Skip to content

Commit

Permalink
AVRO-3403: Improvements after review
Browse files Browse the repository at this point in the history 
Improved the code thanks to good review comments.
  • Loading branch information
@opwvhk
opwvhk committed Sep 8, 2022
1 parent 858957f commit 11fa739
Show file tree
Hide file tree
Showing 16 changed files with 417 additions and 239 deletions.
5 changes: 3 additions & 2 deletions lang/java/archetypes/avro-service-archetype/src/main/pom/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@
<name>Simple Avro Ordering Service</name>

<properties>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
<maven.compiler.source>${maven.compiler.source}</maven.compiler.source>
<maven.compiler.target>${maven.compiler.target}</maven.compiler.target>
<project.build.sourceEncoding>${project.build.sourceEncoding}</project.build.sourceEncoding>
<avro.version>${project.version}</avro.version>
<jackson-bom.version>${jackson-bom.version}</jackson-bom.version>
<junit.version>${junit.version}</junit.version>
Expand Down
67 changes: 38 additions & 29 deletions lang/java/compiler/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,37 +182,8 @@
</executions>
</plugin>
</plugins>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.eclipse.m2e</groupId>
<artifactId>lifecycle-mapping</artifactId>
<version>1.0.0</version>
<configuration>
<lifecycleMappingMetadata>
<pluginExecutions>
<pluginExecution>
<pluginExecutionFilter>
<groupId>org.codehaus.mojo</groupId>
<artifactId>exec-maven-plugin</artifactId>
<versionRange>[1.0,)</versionRange>
<goals>
<goal>exec</goal>
</goals>
</pluginExecutionFilter>
<action>
<execute/>
</action>
</pluginExecution>
</pluginExecutions>
</lifecycleMappingMetadata>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>


<dependencies>
<dependency>
<groupId>${project.groupId}</groupId>
Expand Down Expand Up @@ -241,4 +212,42 @@
</dependency>
</dependencies>

<profiles>
<profile>
<id>m2e</id>
<activation>
<property><name>m2e.version</name></property>
</activation>
<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.eclipse.m2e</groupId>
<artifactId>lifecycle-mapping</artifactId>
<version>1.0.0</version>
<configuration>
<lifecycleMappingMetadata>
<pluginExecutions>
<pluginExecution>
<pluginExecutionFilter>
<groupId>org.codehaus.mojo</groupId>
<artifactId>exec-maven-plugin</artifactId>
<versionRange>[1.0,)</versionRange>
<goals>
<goal>exec</goal>
</goals>
</pluginExecutionFilter>
<action>
<execute/>
</action>
</pluginExecution>
</pluginExecutions>
</lifecycleMappingMetadata>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>
</profile>
</profiles>
</project>
68 changes: 39 additions & 29 deletions lang/java/idl/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,37 +107,8 @@
</configuration>
</plugin>
</plugins>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.eclipse.m2e</groupId>
<artifactId>lifecycle-mapping</artifactId>
<version>1.0.0</version>
<configuration>
<lifecycleMappingMetadata>
<pluginExecutions>
<pluginExecution>
<pluginExecutionFilter>
<groupId>org.codehaus.mojo</groupId>
<artifactId>exec-maven-plugin</artifactId>
<versionRange>[1.0,)</versionRange>
<goals>
<goal>exec</goal>
</goals>
</pluginExecutionFilter>
<action>
<execute/>
</action>
</pluginExecution>
</pluginExecutions>
</lifecycleMappingMetadata>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>


<dependencies>
<dependency>
<groupId>${project.groupId}</groupId>
Expand All @@ -159,4 +130,43 @@
<artifactId>jackson-databind</artifactId>
</dependency>
</dependencies>

<profiles>
<profile>
<id>m2e</id>
<activation>
<property><name>m2e.version</name></property>
</activation>
<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.eclipse.m2e</groupId>
<artifactId>lifecycle-mapping</artifactId>
<version>1.0.0</version>
<configuration>
<lifecycleMappingMetadata>
<pluginExecutions>
<pluginExecution>
<pluginExecutionFilter>
<groupId>org.codehaus.mojo</groupId>
<artifactId>exec-maven-plugin</artifactId>
<versionRange>[1.0,)</versionRange>
<goals>
<goal>exec</goal>
</goals>
</pluginExecutionFilter>
<action>
<execute/>
</action>
</pluginExecution>
</pluginExecutions>
</lifecycleMappingMetadata>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>
</profile>
</profiles>
</project>
8 changes: 4 additions & 4 deletions lang/java/idl/src/main/java/org/apache/avro/idl/IdlFile.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public List<String> getWarnings() {

public List<String> getWarnings(String importFile) {
return warnings.stream()
.map(warning -> importFile + " " + Character.toLowerCase(warning.charAt(0)) + warning.substring(1))
.map(warning -> importFile + ' ' + Character.toLowerCase(warning.charAt(0)) + warning.substring(1))
.collect(Collectors.toList());
}

Expand Down Expand Up @@ -96,7 +96,7 @@ public Schema getNamedSchema(String name) {
return result;
}
if (namespace != null && !name.contains(".")) {
result = namedSchemas.get(namespace + "." + name);
result = namedSchemas.get(namespace + '.' + name);
}
return result;
}
Expand All @@ -111,9 +111,9 @@ String outputString() {
} else {
StringBuilder buffer = new StringBuilder();
for (Schema schema : namedSchemas.values()) {
buffer.append(",").append(schema);
buffer.append(',').append(schema);
}
buffer.append("]").setCharAt(0, '[');
buffer.append(']').setCharAt(0, '[');
return buffer.toString();
}
}
Expand Down
3 changes: 1 addition & 2 deletions lang/java/idl/src/main/java/org/apache/avro/idl/Schemas.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,14 @@ public static <T> T visit(final Schema start, final SchemaVisitor<T> visitor) {
switch (action) {
case CONTINUE:
break;
case SKIP_SUBTREE:
throw new UnsupportedOperationException();
case SKIP_SIBLINGS:
while (dq.peek() instanceof Schema) {
dq.remove();
}
break;
case TERMINATE:
return visitor.get();
case SKIP_SUBTREE:
default:
throw new UnsupportedOperationException("Invalid action " + action);
}
Expand Down
24 changes: 13 additions & 11 deletions lang/java/idl/src/test/idl/input/forward_ref.avdl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,19 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

@namespace("org.foo")
protocol Import {
/* Name Value record */
record ANameValue {
/** the name */
string name;
/** the value */
string value;
/* is the value a json object */
ValueType type = "PLAIN";
}
enum ValueType {JSON, BASE64BIN, PLAIN}
/* Name Value record */
record ANameValue {
/** the name */
string name;
/** the value */
string value;
/* is the value a json object */
ValueType type = "PLAIN";
}

enum ValueType {
JSON, BASE64BIN, PLAIN
}
}
93 changes: 71 additions & 22 deletions lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/AbstractAvroMojo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@

package org.apache.avro.mojo;

import org.apache.avro.LogicalTypes;
import org.apache.avro.Protocol;
import org.apache.avro.Schema;
import org.apache.avro.compiler.specific.SpecificCompiler;
import org.apache.avro.generic.GenericData;
import org.apache.maven.artifact.DependencyResolutionRequiredException;
import org.apache.maven.plugin.AbstractMojo;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.project.MavenProject;
import org.apache.maven.shared.model.fileset.FileSet;
import org.apache.maven.shared.model.fileset.util.FileSetManager;

import java.io.File;
import java.io.IOException;
import java.lang.reflect.InvocationTargetException;
Expand All @@ -28,15 +40,6 @@
import java.util.Arrays;
import java.util.List;

import org.apache.avro.LogicalTypes;
import org.apache.avro.compiler.specific.SpecificCompiler;
import org.apache.maven.artifact.DependencyResolutionRequiredException;
import org.apache.maven.plugin.AbstractMojo;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.project.MavenProject;
import org.apache.maven.shared.model.fileset.FileSet;
import org.apache.maven.shared.model.fileset.util.FileSetManager;

/**
* Base for Avro Compiler Mojos.
*/
Expand Down Expand Up @@ -274,14 +277,23 @@ private String[] getIncludedFiles(String absPath, String[] excludes, String[] in
}

private void compileFiles(String[] files, File sourceDir, File outDir) throws MojoExecutionException {
for (String filename : files) {
try {
// Need to register custom logical type factories before schema compilation.
loadLogicalTypesFactories();
doCompile(filename, sourceDir, outDir);
} catch (IOException e) {
throw new MojoExecutionException("Error compiling protocol file " + filename + " to " + outDir, e);
final ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
try {
Thread.currentThread().setContextClassLoader(createClassLoader());

for (String filename : files) {
try {
// Need to register custom logical type factories before schema compilation.
loadLogicalTypesFactories();
doCompile(filename, sourceDir, outDir);
} catch (IOException e) {
throw new MojoExecutionException("Error compiling protocol file " + filename + " to " + outDir, e);
}
}
} catch (MalformedURLException | DependencyResolutionRequiredException e) {
throw new MojoExecutionException("Cannot locate classpath entries", e);
} finally {
Thread.currentThread().setContextClassLoader(contextClassLoader);
}
}

Expand Down Expand Up @@ -314,7 +326,7 @@ protected List<Object> instantiateAdditionalVelocityTools() {
final List<Object> velocityTools = new ArrayList<>(velocityToolsClassesNames.length);
for (String velocityToolClassName : velocityToolsClassesNames) {
try {
Class klass = Class.forName(velocityToolClassName);
Class<?> klass = Class.forName(velocityToolClassName);
velocityTools.add(klass.getDeclaredConstructor().newInstance());
} catch (Exception e) {
throw new RuntimeException(e);
Expand All @@ -325,20 +337,57 @@ protected List<Object> instantiateAdditionalVelocityTools() {

protected abstract void doCompile(String filename, File sourceDirectory, File outputDirectory) throws IOException;

protected URLClassLoader createClassLoader() throws DependencyResolutionRequiredException, MalformedURLException {
protected void doCompile(File sourceFileForModificationDetection, Schema schema, File outputDirectory)
throws IOException {
doCompile(sourceFileForModificationDetection, new SpecificCompiler(schema), outputDirectory);
}

protected void doCompile(File sourceFileForModificationDetection, Protocol protocol, File outputDirectory)
throws IOException {
doCompile(sourceFileForModificationDetection, new SpecificCompiler(protocol), outputDirectory);
}

private void doCompile(File sourceFileForModificationDetection, SpecificCompiler compiler, File outputDirectory)
throws IOException {
compiler.setTemplateDir(templateDirectory);
compiler.setStringType(GenericData.StringType.valueOf(stringType));
compiler.setFieldVisibility(getFieldVisibility());
compiler.setCreateOptionalGetters(createOptionalGetters);
compiler.setGettersReturnOptional(gettersReturnOptional);
compiler.setOptionalGettersForNullableFieldsOnly(optionalGettersForNullableFieldsOnly);
compiler.setCreateSetters(createSetters);
compiler.setEnableDecimalLogicalType(enableDecimalLogicalType);
try {
final URLClassLoader classLoader = createClassLoader();
for (String customConversion : customConversions) {
compiler.addCustomConversion(classLoader.loadClass(customConversion));
}
} catch (ClassNotFoundException | DependencyResolutionRequiredException e) {
throw new IOException(e);
}
compiler.setOutputCharacterEncoding(project.getProperties().getProperty("project.build.sourceEncoding"));
compiler.setAdditionalVelocityTools(instantiateAdditionalVelocityTools());
compiler.compileToDestination(sourceFileForModificationDetection, outputDirectory);
}

protected List<URL> findClasspath() throws DependencyResolutionRequiredException, MalformedURLException {
final List<URL> urls = appendElements(project.getRuntimeClasspathElements());
urls.addAll(appendElements(project.getTestClasspathElements()));
return urls;
}

protected URLClassLoader createClassLoader() throws DependencyResolutionRequiredException, MalformedURLException {
final List<URL> urls = findClasspath();
return new URLClassLoader(urls.toArray(new URL[0]), Thread.currentThread().getContextClassLoader());
}

private List<URL> appendElements(List runtimeClasspathElements) throws MalformedURLException {
private List<URL> appendElements(List<String> runtimeClasspathElements) throws MalformedURLException {
if (runtimeClasspathElements == null) {
return new ArrayList<>();
}
List<URL> runtimeUrls = new ArrayList<>(runtimeClasspathElements.size());
for (Object runtimeClasspathElement : runtimeClasspathElements) {
String element = (String) runtimeClasspathElement;
runtimeUrls.add(new File(element).toURI().toURL());
for (String runtimeClasspathElement : runtimeClasspathElements) {
runtimeUrls.add(new File(runtimeClasspathElement).toURI().toURL());
}
return runtimeUrls;
}
Expand Down
Loading

0 comments on commit 11fa739

Please sign in to comment.