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

ArC - improve error message when annotation default value cannot be read #1838

Merged
merged 3 commits into from
Apr 4, 2019
Merged

ArC - improve error message when annotation default value cannot be read #1838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f7c9614
ArC - improve error message when annotation default value cannot be read
mkouba Apr 3, 2019
efa1fbb
Use IllegalStateException instead of NPE
mkouba Apr 3, 2019
9b4520f
Re-index application dependency if indexed with old Jandex version
mkouba Apr 3, 2019
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
46 changes: 32 additions & 14 deletions core/deployment/src/main/java/io/quarkus/deployment/index/ApplicationArchiveBuildStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -64,6 +63,9 @@ public class ApplicationArchiveBuildStep {

private static final String JANDEX_INDEX = "META-INF/jandex.idx";

// At least Jandex 2.1 is needed
private static final int REQUIRED_INDEX_VERSION = 8;

IndexDependencyConfiguration config;

@ConfigRoot(phase = ConfigPhase.BUILD_TIME)
Expand Down Expand Up @@ -162,7 +164,6 @@ private static Collection<? extends Path> getMarkerFilePaths(ClassLoader classLo
ret.add(urlToPath(url));
}
}

return ret;
}

Expand All @@ -187,11 +188,19 @@ private static Index handleFilePath(Path path) throws IOException {
Path existing = path.resolve(JANDEX_INDEX);
if (Files.exists(existing)) {
try (FileInputStream in = new FileInputStream(existing.toFile())) {
IndexReader r = new IndexReader(in);
return r.read();
IndexReader reader = new IndexReader(in);
if (reader.getIndexVersion() < REQUIRED_INDEX_VERSION) {
LOGGER.warnf("Re-indexing %s - at least Jandex 2.1 must be used to index an application dependency", path);
return indexFilePath(path);
} else {
return reader.read();
}
}
}
return indexFilePath(path);
}

private static Index indexFilePath(Path path) throws IOException {
Indexer indexer = new Indexer();
try (Stream<Path> stream = Files.walk(path)) {
stream.forEach(path1 -> {
Expand All @@ -208,23 +217,32 @@ private static Index handleFilePath(Path path) throws IOException {
}

private static Index handleJarPath(Path path) throws IOException {
Indexer indexer = new Indexer();
try (JarFile file = new JarFile(path.toFile())) {
ZipEntry existing = file.getEntry(JANDEX_INDEX);
if (existing != null) {
try (InputStream in = file.getInputStream(existing)) {
IndexReader r = new IndexReader(in);
return r.read();
IndexReader reader = new IndexReader(in);
if (reader.getIndexVersion() < REQUIRED_INDEX_VERSION) {
LOGGER.warnf("Re-indexing %s - at least Jandex 2.1 must be used to index an application dependency",
path);
return indexJar(file);
} else {
return reader.read();
}
}
}
return indexJar(file);
}
}

Enumeration<JarEntry> e = file.entries();
while (e.hasMoreElements()) {
JarEntry entry = e.nextElement();
if (entry.getName().endsWith(".class")) {
try (InputStream inputStream = file.getInputStream(entry)) {
indexer.index(inputStream);
}
private static Index indexJar(JarFile file) throws IOException {
Indexer indexer = new Indexer();
Enumeration<JarEntry> e = file.entries();
while (e.hasMoreElements()) {
JarEntry entry = e.nextElement();
if (entry.getName().endsWith(".class")) {
try (InputStream inputStream = file.getInputStream(entry)) {
indexer.index(inputStream);
}
}
}
Expand Down
50 changes: 6 additions & 44 deletions ...ects/arc/processor/src/main/java/io/quarkus/arc/processor/AnnotationLiteralGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.DotName;
import org.jboss.jandex.MethodInfo;
import org.jboss.jandex.PrimitiveType;
import org.jboss.jandex.Type;
import org.jboss.logging.Logger;

Expand Down Expand Up @@ -114,14 +113,10 @@ static void createSharedAnnotationLiteral(ClassOutput classOutput, Key key, Lite
}

static void createAnnotationLiteral(ClassOutput classOutput, ClassInfo annotationClass,
AnnotationInstance annotationInstance, String literalName) {
createAnnotationLiteral(classOutput, annotationClass, annotationInstance.values(), literalName);
}

static void createAnnotationLiteral(ClassOutput classOutput, ClassInfo annotationClass, List<AnnotationValue> values,
AnnotationInstance annotationInstance,
String literalName) {

Map<String, AnnotationValue> annotationValues = values.stream()
Map<String, AnnotationValue> annotationValues = annotationInstance.values().stream()
.collect(Collectors.toMap(AnnotationValue::name, Function.identity()));

// Ljavax/enterprise/util/AnnotationLiteral<Lcom/foo/MyQualifier;>;Lcom/foo/MyQualifier;
Expand All @@ -142,45 +137,12 @@ static void createAnnotationLiteral(ClassOutput classOutput, ClassInfo annotatio
if (value == null) {
value = method.defaultValue();
}
ResultHandle retValue = null;
if (value == null) {
switch (method.returnType().kind()) {
case CLASS:
case ARRAY:
retValue = valueMethod.loadNull();
break;
case PRIMITIVE:
PrimitiveType primitiveType = method.returnType().asPrimitiveType();
switch (primitiveType.primitive()) {
case BOOLEAN:
retValue = valueMethod.load(false);
break;
case BYTE:
case SHORT:
case INT:
retValue = valueMethod.load(0);
break;
case LONG:
retValue = valueMethod.load(0L);
break;
case FLOAT:
retValue = valueMethod.load(0.0f);
break;
case DOUBLE:
retValue = valueMethod.load(0.0d);
break;
case CHAR:
retValue = valueMethod.load('\u0000');
break;
}
break;
default:
break;
}
} else {
retValue = loadValue(valueMethod, value, annotationClass, method);
throw new IllegalStateException(String.format(
"Value is not set for %s.%s(). Most probably an older version of Jandex was used to index an application dependency. Make sure that Jandex 2.1+ is used.",
method.declaringClass().name(), method.name()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we don't need to handle the primitive case anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do handle primitives in AnnotationLiteralGenerator.loadValue(). I've just removed the part which sets the values to the default JLS values - this wouldn't help much and the produced literal would be incorrect.

}
valueMethod.returnValue(retValue);
valueMethod.returnValue(loadValue(valueMethod, value, annotationClass, method));
}
annotationLiteral.close();
LOGGER.debugf("Annotation literal generated: %s", literalName);
Expand Down
4 changes: 3 additions & 1 deletion ...ects/arc/processor/src/main/java/io/quarkus/arc/processor/AnnotationLiteralProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ ResultHandle process(BytecodeCreator bytecode, ClassOutput classOutput, ClassInf
value = method.defaultValue();
}
if (value == null) {
throw new NullPointerException("Value not set for " + method);
throw new IllegalStateException(String.format(
"Value is not set for %s.%s(). Most probably an older version of Jandex was used to index an application dependency. Make sure that Jandex 2.1+ is used.",
method.declaringClass().name(), method.name()));
}
ResultHandle retValue = AnnotationLiteralGenerator.loadValue(bytecode, value, annotationClass, method);
constructorParams[iterator.previousIndex()] = retValue;
Expand Down