From 70eae7ec2d323d918c6d1642f0773f040574b412 Mon Sep 17 00:00:00 2001 From: BJ Hargrave Date: Mon, 7 Feb 2022 14:35:19 -0500 Subject: [PATCH] bundle annotations: Support replacement of enum types Using enum types in the CLASS retention bundle annotations is causing issues for those using the annotations. Various tools, such as javadoc, attempt to reify the elements in the annotations and since the osgi.annotation jar is generally a scope=provided dependency, the enum types are not available to downstream users of the jars using the OSGi annotations and so tools generates an annoying warning. See https://github.com/quarkusio/quarkus/issues/19970 and https://github.com/eclipse/microprofile-config/issues/716. We support the use of enum values or string values as the annotation element value through existing conversion support. The OSGi change in https://github.com/osgi/osgi/pull/404 will move from using enum values to use string values which are equivalent to the former enum value names. We seamlessly handle old and new annotations using the old enum values or the new string values. Prior to this fix, Bnd already handled the change through the Converter which converts the string value to an internal enum value. This change avoids the internal enum type and processes the string value or string name of the enum value when processing older versions of the OSGi annotations. Signed-off-by: BJ Hargrave --- .../std/a/ResolutionDirectiveOverride.java | 6 +- .../std/b/CardinalityDirectiveOverride.java | 6 +- .../aQute/bnd/bundle/annotations/Export.java | 40 +------------ .../bnd/bundle/annotations/Requirement.java | 58 +------------------ .../src/aQute/bnd/osgi/Analyzer.java | 4 +- .../src/aQute/bnd/osgi/AnnotationHeaders.java | 53 ++++++++++++++--- 6 files changed, 59 insertions(+), 108 deletions(-) diff --git a/biz.aQute.bndlib.tests/test/test/annotationheaders/attrs/std/a/ResolutionDirectiveOverride.java b/biz.aQute.bndlib.tests/test/test/annotationheaders/attrs/std/a/ResolutionDirectiveOverride.java index 657fc014c1..e46145a9b7 100644 --- a/biz.aQute.bndlib.tests/test/test/annotationheaders/attrs/std/a/ResolutionDirectiveOverride.java +++ b/biz.aQute.bndlib.tests/test/test/annotationheaders/attrs/std/a/ResolutionDirectiveOverride.java @@ -3,9 +3,9 @@ import org.osgi.annotation.bundle.Attribute; import org.osgi.annotation.bundle.Directive; import org.osgi.annotation.bundle.Requirement; -import org.osgi.annotation.bundle.Requirement.Resolution; +import org.osgi.resource.Namespace; -@Custom(name = "bar", resolution = Resolution.OPTIONAL) +@Custom(name = "bar", resolution = Namespace.RESOLUTION_OPTIONAL) public class ResolutionDirectiveOverride {} @Requirement(namespace = "foo") @@ -14,5 +14,5 @@ public class ResolutionDirectiveOverride {} String name(); @Directive - Resolution resolution() default Resolution.MANDATORY; + String resolution() default Namespace.RESOLUTION_MANDATORY; } diff --git a/biz.aQute.bndlib.tests/test/test/annotationheaders/attrs/std/b/CardinalityDirectiveOverride.java b/biz.aQute.bndlib.tests/test/test/annotationheaders/attrs/std/b/CardinalityDirectiveOverride.java index d48b208518..53d93c86b9 100644 --- a/biz.aQute.bndlib.tests/test/test/annotationheaders/attrs/std/b/CardinalityDirectiveOverride.java +++ b/biz.aQute.bndlib.tests/test/test/annotationheaders/attrs/std/b/CardinalityDirectiveOverride.java @@ -3,9 +3,9 @@ import org.osgi.annotation.bundle.Attribute; import org.osgi.annotation.bundle.Directive; import org.osgi.annotation.bundle.Requirement; -import org.osgi.annotation.bundle.Requirement.Cardinality; +import org.osgi.resource.Namespace; -@Custom(name = "bar", cardinality = Cardinality.MULTIPLE) +@Custom(name = "bar", cardinality = Namespace.CARDINALITY_MULTIPLE) public class CardinalityDirectiveOverride {} @Requirement(namespace = "foo") @@ -14,5 +14,5 @@ public class CardinalityDirectiveOverride {} String name(); @Directive - Cardinality cardinality() default Cardinality.SINGLE; + String cardinality() default Namespace.CARDINALITY_SINGLE; } diff --git a/biz.aQute.bndlib/src/aQute/bnd/bundle/annotations/Export.java b/biz.aQute.bndlib/src/aQute/bnd/bundle/annotations/Export.java index e53b114f85..df2d71ffec 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/bundle/annotations/Export.java +++ b/biz.aQute.bndlib/src/aQute/bnd/bundle/annotations/Export.java @@ -22,8 +22,6 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -import org.osgi.annotation.versioning.ConsumerType; -import org.osgi.annotation.versioning.ProviderType; import org.osgi.annotation.versioning.Version; /** @@ -76,40 +74,8 @@ * bundles are willing to import exported packages; these imports will allow * a framework to substitute exports for imports. *

- * If not specified, the {@link Substitution#CALCULATED} substitution policy - * is used for this package. + * If not specified, the {@code CALCULATED} substitution policy is used for + * this package. */ - Substitution substitution() default Substitution.CALCULATED; - - /** - * Substitution policy for this package. - */ - public enum Substitution { - /** - * Use a consumer type version range for the import package clause when - * substitutably importing a package. - * - * @see ConsumerType - */ - CONSUMER, - - /** - * Use a provider type version range for the import package clause when - * substitutably importing a package. - * - * @see ProviderType - */ - PROVIDER, - - /** - * The package must not be substitutably imported. - */ - NOIMPORT, - - /** - * The policy value is calculated by inspection of the classes in the - * package. - */ - CALCULATED - } + String substitution() default "CALCULATED"; } diff --git a/biz.aQute.bndlib/src/aQute/bnd/bundle/annotations/Requirement.java b/biz.aQute.bndlib/src/aQute/bnd/bundle/annotations/Requirement.java index 9a75accfdc..4142eeaab7 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/bundle/annotations/Requirement.java +++ b/biz.aQute.bndlib/src/aQute/bnd/bundle/annotations/Requirement.java @@ -117,33 +117,7 @@ * If not specified, the {@code cardinality} directive is omitted from the * requirement clause. */ - Cardinality cardinality() default Cardinality.SINGLE; - - /** - * Cardinality for this requirement. - */ - public enum Cardinality { - /** - * Indicates if the requirement can only be wired a single time. - */ - SINGLE("single"), // Namespace.CARDINALITY_SINGLE - - /** - * Indicates if the requirement can be wired multiple times. - */ - MULTIPLE("multiple"); // Namespace.CARDINALITY_MULTIPLE - - private final String value; - - Cardinality(String value) { - this.value = value; - } - - @Override - public String toString() { - return value; - } - } + String cardinality() default "SINGLE"; /** * The resolution policy of this requirement. @@ -155,33 +129,5 @@ public String toString() { * If not specified, the {@code resolution} directive is omitted from the * requirement clause. */ - Resolution resolution() default Resolution.MANDATORY; - - /** - * Resolution for this requirement. - */ - public enum Resolution { - /** - * A mandatory requirement forbids the bundle to resolve when the - * requirement is not satisfied. - */ - MANDATORY("mandatory"), // Namespace.RESOLUTION_MANDATORY - - /** - * An optional requirement allows a bundle to resolve even if the - * requirement is not satisfied. - */ - OPTIONAL("optional"); // Namespace.RESOLUTION_OPTIONAL - - private final String value; - - Resolution(String value) { - this.value = value; - } - - @Override - public String toString() { - return value; - } - } + String resolution() default "MANDATORY"; } diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java index 9d9ac8efc0..52b4495ccb 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java @@ -27,6 +27,7 @@ import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; @@ -841,7 +842,8 @@ public void annotation(Annotation a) { Object substitution = a.get("substitution"); if (substitution != null) { - switch (substitution.toString()) { + switch (substitution.toString() + .toUpperCase(Locale.ROOT)) { case "CONSUMER" : info.put(PROVIDE_DIRECTIVE, "false"); break; diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/AnnotationHeaders.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/AnnotationHeaders.java index 368eb37150..0f75723328 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/osgi/AnnotationHeaders.java +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/AnnotationHeaders.java @@ -10,6 +10,7 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map.Entry; import java.util.Optional; import java.util.Set; @@ -18,6 +19,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import org.osgi.resource.Namespace; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -34,6 +36,7 @@ import aQute.bnd.bundle.annotations.Headers; import aQute.bnd.bundle.annotations.Requirement; import aQute.bnd.bundle.annotations.Requirements; +import aQute.bnd.exceptions.Exceptions; import aQute.bnd.header.Attrs; import aQute.bnd.header.OSGiHeader; import aQute.bnd.header.Parameters; @@ -42,13 +45,12 @@ import aQute.bnd.osgi.Descriptors.PackageRef; import aQute.bnd.osgi.Descriptors.TypeRef; import aQute.bnd.stream.MapStream; +import aQute.bnd.unmodifiable.Sets; import aQute.bnd.version.Version; import aQute.bnd.version.VersionRange; import aQute.lib.collections.MultiMap; import aQute.lib.converter.Converter; -import aQute.bnd.exceptions.Exceptions; import aQute.lib.strings.Strings; -import aQute.bnd.unmodifiable.Sets; /** * This class parses the 'header annotations'. Header annotations are @@ -740,23 +742,58 @@ private void doRequirement(Annotation a, Requirement annotation) throws Exceptio "The Requirement annotation with namespace %s applied to class %s has invalid filter information.", annotation.namespace(), current.getFQN()); } - req.append(";filter:='") + req.append(';') + .append(Namespace.REQUIREMENT_FILTER_DIRECTIVE) + .append(':') + .append('=') + .append('\'') .append(filter) .append('\''); } if (a.containsKey("resolution")) { - req.append(";resolution:=") - .append(annotation.resolution()); + String resolution = annotation.resolution() + .toLowerCase(Locale.ROOT); + switch (resolution) { + case Namespace.RESOLUTION_MANDATORY : + case Namespace.RESOLUTION_OPTIONAL : + req.append(';') + .append(Namespace.REQUIREMENT_RESOLUTION_DIRECTIVE) + .append(':') + .append('=') + .append(resolution); + break; + default : + analyzer.error("Requirement annotation in %s has invalid resolution value: %s", current, + annotation.resolution()); + break; + } } if (a.containsKey("cardinality")) { - req.append(";cardinality:=") - .append(annotation.cardinality()); + String cardinality = annotation.cardinality() + .toLowerCase(Locale.ROOT); + switch (cardinality) { + case Namespace.CARDINALITY_SINGLE : + case Namespace.CARDINALITY_MULTIPLE : + req.append(';') + .append(Namespace.REQUIREMENT_CARDINALITY_DIRECTIVE) + .append(':') + .append('=') + .append(cardinality); + break; + default : + analyzer.error("Requirement annotation in %s has invalid cardinality value: %s", current, + annotation.cardinality()); + break; + } } if (a.containsKey("effective")) { - req.append(";effective:="); + req.append(';') + .append(Namespace.REQUIREMENT_EFFECTIVE_DIRECTIVE) + .append(':') + .append('='); escape(req, annotation.effective()); }