You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/03/11 12:56:32 UTC
[GitHub] [ignite-3] rpuch commented on a change in pull request #714: IGNITE-16674 Fix incremental compilation for network messages
rpuch commented on a change in pull request #714:
URL: https://github.com/apache/ignite-3/pull/714#discussion_r824562664
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/IncrementalCompilationConfig.java
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.ignite.internal.network.processor;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.squareup.javapoet.ClassName;
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Reader;
+import java.nio.file.NoSuchFileException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import javax.annotation.processing.Filer;
+import javax.annotation.processing.ProcessingEnvironment;
+import javax.tools.FileObject;
+import javax.tools.StandardLocation;
+
+/**
+ * Incremental configuration of the {@link TransferableObjectProcessor}.
+ * Holds data between (re-)compilations.
+ */
+class IncrementalCompilationConfig {
+ /** Incremental compilation configuration file name. */
+ static final String CONFIG_FILE_NAME = "META-INF/transferable.messages";
+
+ /** Message group class name. */
+ private ClassName messageGroupClassName;
+
+ /** Messages. */
+ private final List<ClassName> messageClasses;
+
+ IncrementalCompilationConfig(ClassName messageGroupClassName, List<ClassName> messageClasses) {
+ this.messageGroupClassName = messageGroupClassName;
+ this.messageClasses = messageClasses;
+ }
+
+ /**
+ * Saves configuration on disk.
+ *
+ * @param processingEnv Processing environment.
+ */
+ void writeConfig(ProcessingEnvironment processingEnv) {
+ Filer filer = processingEnv.getFiler();
+
+ FileObject fileObject;
+ try {
+ fileObject = filer.createResource(StandardLocation.CLASS_OUTPUT, "", CONFIG_FILE_NAME);
+ } catch (IOException e) {
+ throw new ProcessingException(e.getMessage());
+ }
+
+ try (OutputStream out = fileObject.openOutputStream()) {
+ BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(out, UTF_8));
Review comment:
How about including the `BufferedWriter` construction in the try-with-resources expression? In such a case `flush()` is not needed, and also it seems to be more idiomatic and bullet-proof.
##########
File path: modules/network/src/integrationTest/java/org/apache/ignite/internal/network/processor/InMemoryJavaFileManager.java
##########
@@ -0,0 +1,241 @@
+/*
+ * 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.ignite.internal.network.processor;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.io.ByteSource;
+import com.google.testing.compile.ForwardingStandardJavaFileManager;
+import java.io.ByteArrayOutputStream;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Reader;
+import java.io.StringWriter;
+import java.io.Writer;
+import java.net.URI;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import javax.tools.FileObject;
+import javax.tools.JavaFileObject;
+import javax.tools.JavaFileObject.Kind;
+import javax.tools.SimpleJavaFileObject;
+import javax.tools.StandardJavaFileManager;
+import javax.tools.StandardLocation;
+
+/**
+ * A file manager implementation that stores all output in memory.
+ */
+final class InMemoryJavaFileManager extends ForwardingStandardJavaFileManager {
+
+ private final LoadingCache<URI, JavaFileObject> inMemoryFileObjects = CacheBuilder.newBuilder()
+ .build(new CacheLoader<>() {
+ @Override
+ public JavaFileObject load(URI key) {
+ return new InMemoryJavaFileObject(key);
+ }
+ });
+
+ InMemoryJavaFileManager(StandardJavaFileManager fileManager) {
+ super(fileManager);
+ }
+
+ private static URI uriForFileObject(Location location, String packageName, String relativeName) {
+ StringBuilder uri = new StringBuilder("mem:///").append(location.getName()).append('/');
+ if (!packageName.isEmpty()) {
+ uri.append(packageName.replace('.', '/')).append('/');
+ }
+ uri.append(relativeName);
+ return URI.create(uri.toString());
+ }
+
+ private static URI uriForJavaFileObject(Location location, String className, Kind kind) {
+ return URI.create("mem:///" + location.getName() + '/' + className.replace('.', '/') + kind.extension);
+ }
+
+ @Override
+ public boolean isSameFile(FileObject a, FileObject b) {
+ /* This check is less strict than what is typically done by the normal compiler file managers
+ * (e.g. JavacFileManager), but is actually the moral equivalent of what most of the
+ * implementations do anyway. We use this check rather than just delegating to the compiler's
+ * file manager because file objects for tests generally cause IllegalArgumentExceptions. */
+ return a.toUri().equals(b.toUri());
+ }
+
+ @Override
+ public String inferBinaryName(Location location, JavaFileObject file) {
+ if (file instanceof InMemoryJavaFileObject) {
+ if (location == StandardLocation.CLASS_OUTPUT || location == StandardLocation.CLASS_PATH
+ || location == StandardLocation.SOURCE_OUTPUT) {
+ return toBinaryName(file);
+ } else {
+ return null;
+ }
+ }
+ return super.inferBinaryName(location, file);
+ }
+
+ private static String toBinaryName(JavaFileObject file) {
+ String fileName = file.getName();
+ // We are only interested in "org.apache..."
+ return fileName.substring(fileName.indexOf("org"), fileName.lastIndexOf('.')).replace('/', '.');
+ }
+
+ @Override
+ public FileObject getFileForInput(Location location, String packageName, String relativeName) throws IOException {
+ if (location.isOutputLocation()) {
+ return inMemoryFileObjects.getIfPresent(uriForFileObject(location, packageName, relativeName));
+ } else {
+ return super.getFileForInput(location, packageName, relativeName);
+ }
+ }
+
+ @Override
+ public JavaFileObject getJavaFileForInput(Location location, String className, Kind kind) throws IOException {
+ if (location.isOutputLocation()) {
+ return inMemoryFileObjects.getIfPresent(uriForJavaFileObject(location, className, kind));
+ } else {
+ return super.getJavaFileForInput(location, className, kind);
+ }
+ }
+
+ @Override
+ public FileObject getFileForOutput(Location location, String packageName, String relativeName, FileObject sibling) {
+ URI uri = uriForFileObject(location, packageName, relativeName);
+ return inMemoryFileObjects.getUnchecked(uri);
+ }
+
+ @Override
+ public JavaFileObject getJavaFileForOutput(Location location, String className, final Kind kind, FileObject sibling) {
+ URI uri = uriForJavaFileObject(location, className, kind);
+ return inMemoryFileObjects.getUnchecked(uri);
+ }
+
+ @Override
+ public Iterable<JavaFileObject> list(Location location, String packageName, Set<Kind> kinds, boolean recurse) throws IOException {
+ List<JavaFileObject> list = new ArrayList<>();
+
+ super.list(location, packageName, kinds, recurse).forEach(list::add);
Review comment:
How about simplifying a bit:
`List<JavaFileObject> list = new ArrayList<>(super.list(location, packageName, kinds, recurse));`
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/serialization/RegistryInitializerGenerator.java
##########
@@ -58,10 +64,24 @@ public RegistryInitializerGenerator(ProcessingEnvironment processingEnv, Message
/**
* Generates a class for registering all generated {@link MessageSerializationFactory} for the current module.
*
- * @param messageFactories map from a network message to a corresponding {@code MessageSerializationFactory}
+ * @param messages List of message types.
* @return {@code TypeSpec} of the generated registry initializer
*/
- public TypeSpec generateRegistryInitializer(Map<MessageClass, TypeSpec> messageFactories) {
+ public TypeSpec generateRegistryInitializer(List<ClassName> messages) {
+ Elements elementUtils = processingEnv.getElementUtils();
+
+ List<ClassName> autoSerializableMessages = messages.stream().filter(className -> {
+ TypeElement typeElement = elementUtils.getTypeElement(className.canonicalName());
Review comment:
I suggest to extract this block of code to a method like `isAutoSerializable()`
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/IncrementalCompilationConfig.java
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.ignite.internal.network.processor;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.squareup.javapoet.ClassName;
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Reader;
+import java.nio.file.NoSuchFileException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import javax.annotation.processing.Filer;
+import javax.annotation.processing.ProcessingEnvironment;
+import javax.tools.FileObject;
+import javax.tools.StandardLocation;
+
+/**
+ * Incremental configuration of the {@link TransferableObjectProcessor}.
+ * Holds data between (re-)compilations.
+ */
+class IncrementalCompilationConfig {
+ /** Incremental compilation configuration file name. */
+ static final String CONFIG_FILE_NAME = "META-INF/transferable.messages";
+
+ /** Message group class name. */
+ private ClassName messageGroupClassName;
+
+ /** Messages. */
+ private final List<ClassName> messageClasses;
+
+ IncrementalCompilationConfig(ClassName messageGroupClassName, List<ClassName> messageClasses) {
+ this.messageGroupClassName = messageGroupClassName;
+ this.messageClasses = messageClasses;
+ }
+
+ /**
+ * Saves configuration on disk.
+ *
+ * @param processingEnv Processing environment.
+ */
+ void writeConfig(ProcessingEnvironment processingEnv) {
+ Filer filer = processingEnv.getFiler();
+
+ FileObject fileObject;
+ try {
+ fileObject = filer.createResource(StandardLocation.CLASS_OUTPUT, "", CONFIG_FILE_NAME);
+ } catch (IOException e) {
+ throw new ProcessingException(e.getMessage());
+ }
+
+ try (OutputStream out = fileObject.openOutputStream()) {
+ BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(out, UTF_8));
+ writeClassName(writer, messageGroupClassName);
+ writer.newLine();
+
+ for (ClassName messageClassName : messageClasses) {
+ writeClassName(writer, messageClassName);
+ writer.newLine();
+ }
+
+ writer.flush();
+ } catch (IOException e) {
+ throw new ProcessingException(e.getMessage());
+ }
+ }
+
+ /**
+ * Reads configuration from disk.
+ *
+ * @param processingEnv Processing environment.
+ */
+ static IncrementalCompilationConfig readConfig(ProcessingEnvironment processingEnv) {
Review comment:
How about using `Optional` instead of nullables here?
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/IncrementalCompilationConfig.java
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.ignite.internal.network.processor;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.squareup.javapoet.ClassName;
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Reader;
+import java.nio.file.NoSuchFileException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import javax.annotation.processing.Filer;
+import javax.annotation.processing.ProcessingEnvironment;
+import javax.tools.FileObject;
+import javax.tools.StandardLocation;
+
+/**
+ * Incremental configuration of the {@link TransferableObjectProcessor}.
+ * Holds data between (re-)compilations.
+ */
+class IncrementalCompilationConfig {
+ /** Incremental compilation configuration file name. */
+ static final String CONFIG_FILE_NAME = "META-INF/transferable.messages";
+
+ /** Message group class name. */
+ private ClassName messageGroupClassName;
+
+ /** Messages. */
+ private final List<ClassName> messageClasses;
+
+ IncrementalCompilationConfig(ClassName messageGroupClassName, List<ClassName> messageClasses) {
+ this.messageGroupClassName = messageGroupClassName;
+ this.messageClasses = messageClasses;
+ }
+
+ /**
+ * Saves configuration on disk.
+ *
+ * @param processingEnv Processing environment.
+ */
+ void writeConfig(ProcessingEnvironment processingEnv) {
+ Filer filer = processingEnv.getFiler();
+
+ FileObject fileObject;
+ try {
+ fileObject = filer.createResource(StandardLocation.CLASS_OUTPUT, "", CONFIG_FILE_NAME);
+ } catch (IOException e) {
+ throw new ProcessingException(e.getMessage());
+ }
+
+ try (OutputStream out = fileObject.openOutputStream()) {
+ BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(out, UTF_8));
+ writeClassName(writer, messageGroupClassName);
+ writer.newLine();
+
+ for (ClassName messageClassName : messageClasses) {
+ writeClassName(writer, messageClassName);
+ writer.newLine();
+ }
+
+ writer.flush();
+ } catch (IOException e) {
+ throw new ProcessingException(e.getMessage());
+ }
+ }
+
+ /**
+ * Reads configuration from disk.
+ *
+ * @param processingEnv Processing environment.
+ */
+ static IncrementalCompilationConfig readConfig(ProcessingEnvironment processingEnv) {
+ Filer filer = processingEnv.getFiler();
+
+ FileObject resource;
+
+ try {
+ resource = filer.getResource(StandardLocation.CLASS_OUTPUT, "", CONFIG_FILE_NAME);
+ } catch (IOException e) {
+ return null;
+ }
+
+ try (Reader reader = resource.openReader(true)) {
+ BufferedReader bufferedReader = new BufferedReader(reader);
+ String messageClassNameString = bufferedReader.readLine();
+
+ if (messageClassNameString == null) {
+ return null;
+ }
+
+ ClassName messageClassName = readClassName(messageClassNameString);
+
+ List<ClassName> message = new ArrayList<>();
+
+ String line;
+ while ((line = bufferedReader.readLine()) != null) {
+ ClassName className = readClassName(line);
+ message.add(className);
+ }
+
+ return new IncrementalCompilationConfig(messageClassName, message);
+ } catch (FileNotFoundException | NoSuchFileException e) {
+ return null;
+ } catch (IOException e) {
+ throw new ProcessingException(e.getMessage());
+ }
+ }
+
+ /**
+ * Writes class name with all the enclosing classes.
+ *
+ * @param writer Writer.
+ * @param className Class name.
+ * @throws IOException If failed.
+ */
+ private void writeClassName(BufferedWriter writer, ClassName className) throws IOException {
+ writer.write(className.packageName());
+ writer.write(' ');
+
+ List<String> enclosingSimpleNames = new ArrayList<>();
+ ClassName enclosing = className;
+ while ((enclosing = enclosing.enclosingClassName()) != null) {
+ enclosingSimpleNames.add(enclosing.simpleName());
+ }
+ Collections.reverse(enclosingSimpleNames);
+ for (String enclosingSimpleName : enclosingSimpleNames) {
+ writer.write(enclosingSimpleName);
+ writer.write(' ');
+ }
+
+ writer.write(className.simpleName());
+ }
+
+ /**
+ * Reads class name.
+ *
+ * @param line Line.
+ * @return Class name.
+ */
+ static ClassName readClassName(String line) {
+ String[] split = line.split(" ");
+ String packageName = split[0];
+ String simpleName = split[1];
Review comment:
I suggest renaming `simpleName` to `firstSimpleName`, otherwise it is not too clear why we have both `simpleName` AND `simpleNames` at the same time.
##########
File path: modules/network/src/integrationTest/java/org/apache/ignite/internal/network/processor/ItTransferableObjectProcessorIncrementalTest.java
##########
@@ -0,0 +1,352 @@
+/*
+ * 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.ignite.internal.network.processor;
+
+import static org.apache.ignite.internal.network.processor.IncrementalCompilationConfig.CONFIG_FILE_NAME;
+import static org.apache.ignite.internal.network.processor.IncrementalCompilationConfig.readClassName;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import com.google.testing.compile.JavaFileObjects;
+import com.squareup.javapoet.ClassName;
+import java.io.BufferedReader;
+import java.net.URI;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
+import javax.tools.DiagnosticCollector;
+import javax.tools.FileObject;
+import javax.tools.JavaCompiler;
+import javax.tools.JavaCompiler.CompilationTask;
+import javax.tools.JavaFileManager.Location;
+import javax.tools.JavaFileObject;
+import javax.tools.JavaFileObject.Kind;
+import javax.tools.StandardJavaFileManager;
+import javax.tools.StandardLocation;
+import javax.tools.ToolProvider;
+import org.intellij.lang.annotations.Language;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Integration tests for the {@link TransferableObjectProcessor} incremental compilation.
+ */
+public class ItTransferableObjectProcessorIncrementalTest {
+ /**
+ * Package name of the test sources.
+ */
+ private static final String RESOURCE_PACKAGE_NAME = "org.apache.ignite.internal.network.processor";
+
+ /** File manager for incremental compilation. */
+ private InMemoryJavaFileManager fileManager;
+
+ @BeforeEach
+ void setUp() {
+ DiagnosticCollector<JavaFileObject> diagnosticCollector = new DiagnosticCollector<>();
+ JavaCompiler systemJavaCompiler = ToolProvider.getSystemJavaCompiler();
+ StandardJavaFileManager standardFileManager = systemJavaCompiler.getStandardFileManager(diagnosticCollector, Locale.getDefault(),
+ StandardCharsets.UTF_8);
+
+ this.fileManager = new InMemoryJavaFileManager(standardFileManager);
+ }
+
+ @Test
+ public void testIncrementalRemoveTransferable() throws Exception {
+ String testMessageGroup = "MsgGroup";
+ String testMessageGroupName = "GroupName";
+ String testMessageClass = "TestMessage";
+ String testMessageClass2 = "SomeMessage";
+
+ var compilationObjects1 = new ArrayList<JavaFileObject>();
+ JavaFileObject messageGroupObject = createMessageGroup(testMessageGroup, testMessageGroupName);
+ compilationObjects1.add(messageGroupObject);
+ compilationObjects1.add(createTransferable(testMessageClass));
+
+ Map<URI, JavaFileObject> compilation1 = compile(compilationObjects1);
+
+ JavaFileObject messageRegistry1 = compilation1.get(uriForMessagesFile());
+ try (BufferedReader bufferedReader = new BufferedReader(messageRegistry1.openReader(true))) {
+ ClassName messageGroupClass = readClassName(bufferedReader.readLine());
+
+ assertEquals(ClassName.get(RESOURCE_PACKAGE_NAME, testMessageGroup), messageGroupClass);
+
+ ClassName messageClass = readClassName(bufferedReader.readLine());
+
+ assertEquals(ClassName.get(RESOURCE_PACKAGE_NAME, testMessageClass), messageClass);
+
+ assertNull(bufferedReader.readLine());
+ }
+
+ URI factoryUri = uriForJavaClass(
+ StandardLocation.SOURCE_OUTPUT,
+ RESOURCE_PACKAGE_NAME + "." + testMessageGroupName + "Factory",
+ Kind.SOURCE
+ );
+
+ String messageFactory1 = readJavaFileObject(compilation1.get(factoryUri));
+
+ assertTrue(messageFactory1.contains("TestMessageImpl.builder();"));
+
+ List<JavaFileObject> compilationObjects2 = new ArrayList<>();
+ compilationObjects2.add(createNonTransferable(testMessageClass));
+ compilationObjects2.add(createTransferable(testMessageClass2));
+
+ Map<URI, JavaFileObject> compilation2 = compile(compilationObjects2);
+
+ JavaFileObject messageRegistry2 = compilation2.get(uriForMessagesFile());
+ try (BufferedReader bufferedReader = new BufferedReader(messageRegistry2.openReader(true))) {
+ ClassName messageGroupClass = readClassName(bufferedReader.readLine());
+
+ assertEquals(ClassName.get(RESOURCE_PACKAGE_NAME, testMessageGroup), messageGroupClass);
+
+ ClassName messageClass = readClassName(bufferedReader.readLine());
+
+ assertEquals(ClassName.get(RESOURCE_PACKAGE_NAME, testMessageClass2), messageClass);
+
+ assertNull(bufferedReader.readLine());
+ }
+
+ String messageFactory2 = readJavaFileObject(compilation2.get(factoryUri));
+
+ assertFalse(messageFactory2.contains("TestMessageImpl.builder();"));
+ assertTrue(messageFactory2.contains("SomeMessageImpl.builder();"));
+ }
+
+ @Test
+ public void testIncrementalAddTransferable() throws Exception {
+ String testMessageGroup = "MsgGroup";
+ String testMessageGroupName = "GroupName";
+ String testMessageClass = "TestMessage";
+ String testMessageClass2 = "SomeMessage";
+
+ var compilationObjects1 = new ArrayList<JavaFileObject>();
+ JavaFileObject messageGroupObject = createMessageGroup(testMessageGroup, testMessageGroupName);
+ compilationObjects1.add(messageGroupObject);
+ compilationObjects1.add(createTransferable(testMessageClass));
+
+ Map<URI, JavaFileObject> compilation1 = compile(compilationObjects1);
+
+ JavaFileObject messageRegistry1 = compilation1.get(uriForMessagesFile());
+ try (BufferedReader bufferedReader = new BufferedReader(messageRegistry1.openReader(true))) {
+ ClassName messageGroupClass = readClassName(bufferedReader.readLine());
+
+ assertEquals(ClassName.get(RESOURCE_PACKAGE_NAME, testMessageGroup), messageGroupClass);
+
+ ClassName messageClass = readClassName(bufferedReader.readLine());
+
+ assertEquals(ClassName.get(RESOURCE_PACKAGE_NAME, testMessageClass), messageClass);
+
+ assertNull(bufferedReader.readLine());
+ }
+
+ URI factoryUri = uriForJavaClass(
+ StandardLocation.SOURCE_OUTPUT,
+ RESOURCE_PACKAGE_NAME + "." + testMessageGroupName + "Factory",
+ Kind.SOURCE
+ );
+
+ String messageFactory1 = readJavaFileObject(compilation1.get(factoryUri));
+
+ assertTrue(messageFactory1.contains("TestMessageImpl.builder();"));
+
+ List<JavaFileObject> compilationObjects2 = new ArrayList<>();
+ compilationObjects2.add(createTransferable(testMessageClass2));
+
+ Map<URI, JavaFileObject> compilation2 = compile(compilationObjects2);
+
+ JavaFileObject messageRegistry2 = compilation2.get(uriForMessagesFile());
+ try (BufferedReader bufferedReader = new BufferedReader(messageRegistry2.openReader(true))) {
+ ClassName messageGroupClass = readClassName(bufferedReader.readLine());
+
+ assertEquals(ClassName.get(RESOURCE_PACKAGE_NAME, testMessageGroup), messageGroupClass);
+
+ ClassName messageClass = readClassName(bufferedReader.readLine());
+
+ assertEquals(ClassName.get(RESOURCE_PACKAGE_NAME, testMessageClass), messageClass);
+
+ messageClass = readClassName(bufferedReader.readLine());
+
+ assertEquals(ClassName.get(RESOURCE_PACKAGE_NAME, testMessageClass2), messageClass);
+
+ assertNull(bufferedReader.readLine());
+ }
+
+ String messageFactory2 = readJavaFileObject(compilation2.get(factoryUri));
+
+ assertTrue(messageFactory2.contains("TestMessageImpl.builder();"));
+ assertTrue(messageFactory2.contains("SomeMessageImpl.builder();"));
+ }
+
+ @Test
+ public void testChangeMessageGroup() throws Exception {
+ String testMessageGroup = "MsgGroup";
+ String testMessageGroupName = "GroupName";
+ String testMessageGroup2 = "MyNewGroup";
+ String testMessageGroupName2 = "NewGroupName";
+ String testMessageClass = "TestMessage";
+
+ var compilationObjects1 = new ArrayList<JavaFileObject>();
+ compilationObjects1.add(createMessageGroup(testMessageGroup, testMessageGroupName));
+ compilationObjects1.add(createTransferable(testMessageClass));
+
+ Map<URI, JavaFileObject> compilation1 = compile(compilationObjects1);
+
+ JavaFileObject messageRegistry1 = compilation1.get(uriForMessagesFile());
+ try (BufferedReader bufferedReader = new BufferedReader(messageRegistry1.openReader(true))) {
+ ClassName messageGroupClass = readClassName(bufferedReader.readLine());
+
+ assertEquals(ClassName.get(RESOURCE_PACKAGE_NAME, testMessageGroup), messageGroupClass);
+
+ ClassName messageClass = readClassName(bufferedReader.readLine());
+
+ assertEquals(ClassName.get(RESOURCE_PACKAGE_NAME, testMessageClass), messageClass);
+
+ assertNull(bufferedReader.readLine());
+ }
+
+ URI factory1Uri = uriForJavaClass(
+ StandardLocation.SOURCE_OUTPUT,
+ RESOURCE_PACKAGE_NAME + "." + testMessageGroupName + "Factory",
+ Kind.SOURCE
+ );
+
+ String messageFactory1 = readJavaFileObject(compilation1.get(factory1Uri));
+
+ assertTrue(messageFactory1.contains("TestMessageImpl.builder();"));
+
+ List<JavaFileObject> compilationObjects2 = new ArrayList<>();
+ compilationObjects2.add(createMessageGroup(testMessageGroup2, testMessageGroupName2));
+ compilationObjects2.add(createTransferable(testMessageClass));
+
+ Map<URI, JavaFileObject> compilation2 = compile(compilationObjects2);
+
+ JavaFileObject messageRegistry2 = compilation2.get(uriForMessagesFile());
+ try (BufferedReader bufferedReader = new BufferedReader(messageRegistry2.openReader(true))) {
+ ClassName messageGroupClass = readClassName(bufferedReader.readLine());
+
+ assertEquals(ClassName.get(RESOURCE_PACKAGE_NAME, testMessageGroup2), messageGroupClass);
+
+ ClassName messageClass = readClassName(bufferedReader.readLine());
+
+ assertEquals(ClassName.get(RESOURCE_PACKAGE_NAME, testMessageClass), messageClass);
+
+ assertNull(bufferedReader.readLine());
+ }
+
+ URI factory2Uri = uriForJavaClass(
+ StandardLocation.SOURCE_OUTPUT,
+ RESOURCE_PACKAGE_NAME + "." + testMessageGroupName2 + "Factory",
+ Kind.SOURCE
+ );
+
+ String messageFactory2 = readJavaFileObject(compilation2.get(factory2Uri));
+
+ assertTrue(messageFactory2.contains("TestMessageImpl.builder();"));
+ }
+
+ private String readJavaFileObject(JavaFileObject object) throws Exception {
+ StringBuilder builder = new StringBuilder();
+ try (BufferedReader bufferedReader = new BufferedReader(object.openReader(true))) {
+ String line;
+
+ while ((line = bufferedReader.readLine()) != null) {
+ builder.append(line).append("\n");
+ }
+
+ return builder.toString();
+ }
+ }
+
+ private JavaFileObject createTransferable(String className) {
+ @Language("JAVA") String code =
+ "package " + RESOURCE_PACKAGE_NAME + ";\n"
+ + "import org.apache.ignite.network.NetworkMessage;\n"
+ + "import org.apache.ignite.network.annotations.Transferable;\n"
+ + "\n"
+ + "\n"
+ + "@Transferable(value = 0)\n"
+ + "public interface " + className + " extends NetworkMessage {\n"
+ + " String foo();\n"
+ + "}\n";
+ return JavaFileObjects.forSourceString(className, code);
+ }
+
+ private JavaFileObject createMessageGroup(String className, String groupName) {
+ @Language("JAVA") String code =
+ "package " + RESOURCE_PACKAGE_NAME + ";\n"
+ + "\n"
+ + " import org.apache.ignite.network.annotations.MessageGroup;\n"
+ + "\n"
+ + "@MessageGroup(groupType = 1, groupName = \"" + groupName + "\")\n"
+ + "public class " + className + " {\n"
+ + "}\n";
+ return JavaFileObjects.forSourceString(className, code);
+ }
+
+ private JavaFileObject createNonTransferable(String className) {
+ @Language("JAVA") String code =
+ "package " + RESOURCE_PACKAGE_NAME + ";\n"
+ + "import org.apache.ignite.network.NetworkMessage;\n"
+ + "\n"
+ + "\n"
+ + "public interface " + className + " extends NetworkMessage {\n"
+ + " String foo();\n"
+ + "}\n";
+ return JavaFileObjects.forSourceString(className, code);
+ }
+
+ private Map<URI, JavaFileObject> compile(Iterable<? extends JavaFileObject> files) {
+ JavaCompiler systemJavaCompiler = ToolProvider.getSystemJavaCompiler();
+ DiagnosticCollector<JavaFileObject> diagnosticListener = new DiagnosticCollector<>();
+ CompilationTask task = systemJavaCompiler
+ .getTask(null, fileManager, diagnosticListener, Collections.emptyList(), Set.of(), files);
+
+ task.setProcessors(Collections.singleton(new TransferableObjectProcessor()));
+
+ Boolean result = task.call();
+
+ assertTrue(result);
+
+ Iterable<JavaFileObject> generatedFiles = fileManager.getOutputFiles();
+
+ return StreamSupport.stream(generatedFiles.spliterator(), false)
+ .collect(Collectors.toMap(FileObject::toUri, Function.identity()));
+ }
+
+
Review comment:
Extra blank line
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/IncrementalCompilationConfig.java
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.ignite.internal.network.processor;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.squareup.javapoet.ClassName;
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Reader;
+import java.nio.file.NoSuchFileException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import javax.annotation.processing.Filer;
+import javax.annotation.processing.ProcessingEnvironment;
+import javax.tools.FileObject;
+import javax.tools.StandardLocation;
+
+/**
+ * Incremental configuration of the {@link TransferableObjectProcessor}.
+ * Holds data between (re-)compilations.
+ */
+class IncrementalCompilationConfig {
+ /** Incremental compilation configuration file name. */
+ static final String CONFIG_FILE_NAME = "META-INF/transferable.messages";
+
+ /** Message group class name. */
+ private ClassName messageGroupClassName;
+
+ /** Messages. */
+ private final List<ClassName> messageClasses;
+
+ IncrementalCompilationConfig(ClassName messageGroupClassName, List<ClassName> messageClasses) {
+ this.messageGroupClassName = messageGroupClassName;
+ this.messageClasses = messageClasses;
+ }
+
+ /**
+ * Saves configuration on disk.
+ *
+ * @param processingEnv Processing environment.
+ */
+ void writeConfig(ProcessingEnvironment processingEnv) {
+ Filer filer = processingEnv.getFiler();
+
+ FileObject fileObject;
+ try {
+ fileObject = filer.createResource(StandardLocation.CLASS_OUTPUT, "", CONFIG_FILE_NAME);
+ } catch (IOException e) {
+ throw new ProcessingException(e.getMessage());
+ }
+
+ try (OutputStream out = fileObject.openOutputStream()) {
+ BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(out, UTF_8));
+ writeClassName(writer, messageGroupClassName);
+ writer.newLine();
+
+ for (ClassName messageClassName : messageClasses) {
+ writeClassName(writer, messageClassName);
+ writer.newLine();
+ }
+
+ writer.flush();
+ } catch (IOException e) {
+ throw new ProcessingException(e.getMessage());
+ }
+ }
+
+ /**
+ * Reads configuration from disk.
+ *
+ * @param processingEnv Processing environment.
+ */
+ static IncrementalCompilationConfig readConfig(ProcessingEnvironment processingEnv) {
+ Filer filer = processingEnv.getFiler();
+
+ FileObject resource;
+
+ try {
+ resource = filer.getResource(StandardLocation.CLASS_OUTPUT, "", CONFIG_FILE_NAME);
+ } catch (IOException e) {
+ return null;
+ }
+
+ try (Reader reader = resource.openReader(true)) {
+ BufferedReader bufferedReader = new BufferedReader(reader);
+ String messageClassNameString = bufferedReader.readLine();
+
+ if (messageClassNameString == null) {
+ return null;
+ }
+
+ ClassName messageClassName = readClassName(messageClassNameString);
+
+ List<ClassName> message = new ArrayList<>();
Review comment:
`message` -> `messages`?
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/TransferableObjectProcessor.java
##########
@@ -71,24 +74,71 @@ public SourceVersion getSupportedSourceVersion() {
@Override
public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
try {
+ IncrementalCompilationConfig currentConfig = IncrementalCompilationConfig.readConfig(processingEnv);
+
List<MessageClass> messages = annotations.stream()
.map(roundEnv::getElementsAnnotatedWith)
.flatMap(Collection::stream)
.map(TypeElement.class::cast)
.map(e -> new MessageClass(processingEnv, e))
- .collect(Collectors.toList());
+ .collect(toList());
if (messages.isEmpty()) {
return true;
}
- MessageGroupWrapper messageGroup = getMessageGroup(roundEnv);
+ MessageGroupWrapper messageGroup = getMessageGroup(roundEnv, currentConfig);
+
+ if (currentConfig == null) {
+ List<ClassName> messageClassNames = messages.stream()
+ .map(MessageClass::className)
+ .collect(toList());
+
+ currentConfig = new IncrementalCompilationConfig(ClassName.get(messageGroup.element()), messageClassNames);
+ } else {
+ List<ClassName> messageClassesFromConfig = currentConfig.messageClasses();
+
+ for (int i = messageClassesFromConfig.size() - 1; i >= 0; i--) {
Review comment:
I wonder why is the iteration order reversed?
Also, how about replacing the iteration with a loop over an `Iterator` and using its `remove()` method? That would make the code a little more streamlined.
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/IncrementalCompilationConfig.java
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.ignite.internal.network.processor;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.squareup.javapoet.ClassName;
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Reader;
+import java.nio.file.NoSuchFileException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import javax.annotation.processing.Filer;
+import javax.annotation.processing.ProcessingEnvironment;
+import javax.tools.FileObject;
+import javax.tools.StandardLocation;
+
+/**
+ * Incremental configuration of the {@link TransferableObjectProcessor}.
+ * Holds data between (re-)compilations.
+ */
+class IncrementalCompilationConfig {
+ /** Incremental compilation configuration file name. */
+ static final String CONFIG_FILE_NAME = "META-INF/transferable.messages";
+
+ /** Message group class name. */
+ private ClassName messageGroupClassName;
+
+ /** Messages. */
+ private final List<ClassName> messageClasses;
Review comment:
It looks like this is a 'live' list, so changes made from the outside are visible in it. I suggest mentioning this explicitly in the field javadoc, so that a reader understand instantly that it's a live list and not a copy of it.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org