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 quarkusio/quarkus#19970 and eclipse/microprofile-config#716. We support the use of enum values or string values as the annotation element value through existing conversion support. The OSGi change in osgi/osgi#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 <[email protected]>
bjhargrave · Feb 7, 2022 · 70eae7e · 70eae7e
1 parent 70df4e1
commit 70eae7e
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 108 deletions.
diff --git a/...ute.bndlib.tests/test/test/annotationheaders/attrs/std/a/ResolutionDirectiveOverride.java b/...ute.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/...te.bndlib.tests/test/test/annotationheaders/attrs/std/b/CardinalityDirectiveOverride.java b/...te.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
@@ -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.
 	 * <p>
-	 * 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
@@ -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
@@ -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
@@ -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());
 		}