Bug 567252 Requirement resolution can be completely blocked

Extract requirements-from-attribute scanning for reuse. Signed-off-by: eparovyshnaya <elena.parovyshnaya@gmail.com>
eclipse-passage · Sep 23, 2020 · 1aa2d92 · 1aa2d92
1 parent 50a8ccf
commit 1aa2d92
Show file tree

Hide file tree

Showing 6 changed files with 138 additions and 47 deletions.
diff --git a/...lic.equinox/src/org/eclipse/passage/lic/internal/equinox/requirements/BundleManifest.java b/...lic.equinox/src/org/eclipse/passage/lic/internal/equinox/requirements/BundleManifest.java
@@ -43,7 +43,7 @@ public String get() throws LicensingException {
 			throw noManifest();
 		}
 		try (LineNumberReader reader = new LineNumberReader(new InputStreamReader(url.get().openStream()))) {
-			return reader.lines().collect(Collectors.joining());
+			return reader.lines().collect(Collectors.joining("\n")); //$NON-NLS-1$
 		} catch (IOException e) {
 			throw errorOnReading(e);
 		}

diff --git a/.../src/org/eclipse/passage/lic/internal/equinox/requirements/RequirementFromAttributes.java b/.../src/org/eclipse/passage/lic/internal/equinox/requirements/RequirementFromAttributes.java
@@ -0,0 +1,89 @@
+/*******************************************************************************
+ * Copyright (c) 2020 ArSysOp
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * https://www.eclipse.org/legal/epl-2.0/.
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *     ArSysOp - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.passage.lic.internal.equinox.requirements;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Optional;
+import java.util.function.Supplier;
+
+import org.eclipse.osgi.util.NLS;
+import org.eclipse.passage.lic.internal.api.ServiceInvocationResult;
+import org.eclipse.passage.lic.internal.api.diagnostic.Trouble;
+import org.eclipse.passage.lic.internal.api.requirements.Requirement;
+import org.eclipse.passage.lic.internal.api.restrictions.RestrictionLevel;
+import org.eclipse.passage.lic.internal.base.BaseServiceInvocationResult;
+import org.eclipse.passage.lic.internal.base.diagnostic.code.ServiceFailedOnMorsel;
+import org.eclipse.passage.lic.internal.base.requirements.BaseFeature;
+import org.eclipse.passage.lic.internal.base.requirements.BaseRequirement;
+import org.eclipse.passage.lic.internal.base.restrictions.DefaultRestrictionLevel;
+import org.eclipse.passage.lic.internal.base.version.DefaultVersion;
+import org.eclipse.passage.lic.internal.base.version.SafeVersion;
+import org.eclipse.passage.lic.internal.equinox.i18n.EquinoxMessages;
+import org.osgi.framework.Bundle;
+
+/**
+ * Looks for {@linkplain Requirement} declaration in a single
+ * {@code Capability}.
+ * 
+ * @see RequirementsFromBundle
+ * @see BundleRequirements
+ */
+@SuppressWarnings("restriction")
+final class RequirementFromAttributes implements Supplier<ServiceInvocationResult<Collection<Requirement>>> {
+
+	private final Bundle bundle;
+	private final Map<String, Object> attributes;
+
+	public RequirementFromAttributes(Bundle bundle, Map<String, Object> attributes) {
+		this.bundle = bundle;
+		this.attributes = attributes;
+	}
+
+	@Override
+	public ServiceInvocationResult<Collection<Requirement>> get() {
+		Optional<String> feature = new CapabilityLicFeatureId(attributes).get();
+		if (!feature.isPresent()) {
+			return fail(NLS.bind(EquinoxMessages.RequirementsFromCapability_no_feature_id, //
+					new LicCapabilityNamespace().get(), new BundleName(bundle).get()));
+		}
+		return succeed(requirementFromAttributes(feature.get()));
+	}
+
+	private Requirement requirementFromAttributes(String feature) {
+		String version = new CapabilityLicFeatureVersion(attributes).get()//
+				.map(value -> new SafeVersion(value).value())//
+				.orElse(new DefaultVersion().value());
+		String name = new CapabilityLicFeatureName(attributes).get()//
+				.orElse(feature);
+		String provider = new CapabilityLicFeatureProvider(attributes).get()//
+				.orElseGet(new BundleVendor(bundle));
+		RestrictionLevel level = new CapabilityLicFeatureLevel(attributes).get()//
+				.<RestrictionLevel>map(RestrictionLevel.Of::new) //
+				.orElseGet(new DefaultRestrictionLevel());
+		BaseRequirement requirement = new BaseRequirement(//
+				new BaseFeature(feature, version, name, provider), //
+				level, //
+				bundle.getSymbolicName());
+		return requirement;
+	}
+
+	private ServiceInvocationResult<Collection<Requirement>> fail(String explanation) {
+		return new BaseServiceInvocationResult<>(new Trouble(new ServiceFailedOnMorsel(), explanation));
+	}
+
+	private ServiceInvocationResult<Collection<Requirement>> succeed(Requirement requirement) {
+		return new BaseServiceInvocationResult<>(Collections.singleton(requirement));
+	}
+}
diff --git a/.../src/org/eclipse/passage/lic/internal/equinox/requirements/RequirementFromCapability.java b/.../src/org/eclipse/passage/lic/internal/equinox/requirements/RequirementFromCapability.java
@@ -13,7 +13,6 @@
 package org.eclipse.passage.lic.internal.equinox.requirements;
 
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Map;
 import java.util.Optional;
 import java.util.function.Supplier;
@@ -22,14 +21,8 @@
 import org.eclipse.passage.lic.internal.api.ServiceInvocationResult;
 import org.eclipse.passage.lic.internal.api.diagnostic.Trouble;
 import org.eclipse.passage.lic.internal.api.requirements.Requirement;
-import org.eclipse.passage.lic.internal.api.restrictions.RestrictionLevel;
 import org.eclipse.passage.lic.internal.base.BaseServiceInvocationResult;
 import org.eclipse.passage.lic.internal.base.diagnostic.code.ServiceFailedOnMorsel;
-import org.eclipse.passage.lic.internal.base.requirements.BaseFeature;
-import org.eclipse.passage.lic.internal.base.requirements.BaseRequirement;
-import org.eclipse.passage.lic.internal.base.restrictions.DefaultRestrictionLevel;
-import org.eclipse.passage.lic.internal.base.version.DefaultVersion;
-import org.eclipse.passage.lic.internal.base.version.SafeVersion;
 import org.eclipse.passage.lic.internal.equinox.i18n.EquinoxMessages;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.wiring.BundleCapability;
@@ -56,40 +49,11 @@ public RequirementFromCapability(Bundle bundle, BundleCapability capability) {
 	public ServiceInvocationResult<Collection<Requirement>> get() {
 		Optional<Map<String, Object>> attributes = Optional.ofNullable(capability.getAttributes());
 		if (!attributes.isPresent()) {
-			return fail(NLS.bind(EquinoxMessages.RequirementsFromCapability_no_attributes, //
-					capability.getNamespace(), new BundleName(bundle).get()));
+			return new BaseServiceInvocationResult<>(new Trouble(new ServiceFailedOnMorsel(), //
+					NLS.bind(EquinoxMessages.RequirementsFromCapability_no_attributes, //
+							capability.getNamespace(), new BundleName(bundle).get())));
 		}
-		Optional<String> feature = new CapabilityLicFeatureId(attributes.get()).get();
-		if (!feature.isPresent()) {
-			return fail(NLS.bind(EquinoxMessages.RequirementsFromCapability_no_feature_id, //
-					capability.getNamespace(), new BundleName(bundle).get()));
-		}
-		return succeed(requirementFromAttributes(feature.get(), attributes.get()));
-	}
-
-	private Requirement requirementFromAttributes(String feature, Map<String, Object> attributes) {
-		String version = new CapabilityLicFeatureVersion(attributes).get()//
-				.map(value -> new SafeVersion(value).value())//
-				.orElse(new DefaultVersion().value());
-		String name = new CapabilityLicFeatureName(attributes).get()//
-				.orElse(feature);
-		String provider = new CapabilityLicFeatureProvider(attributes).get()//
-				.orElseGet(new BundleVendor(bundle));
-		RestrictionLevel level = new CapabilityLicFeatureLevel(attributes).get()//
-				.<RestrictionLevel>map(RestrictionLevel.Of::new) //
-				.orElseGet(new DefaultRestrictionLevel());
-		BaseRequirement requirement = new BaseRequirement(//
-				new BaseFeature(feature, version, name, provider), //
-				level, //
-				capability.getResource().getBundle().getSymbolicName());
-		return requirement;
+		return new RequirementFromAttributes(bundle, attributes.get()).get();
 	}
 
-	private ServiceInvocationResult<Collection<Requirement>> fail(String explanation) {
-		return new BaseServiceInvocationResult<>(new Trouble(new ServiceFailedOnMorsel(), explanation));
-	}
-
-	private ServiceInvocationResult<Collection<Requirement>> succeed(Requirement requirement) {
-		return new BaseServiceInvocationResult<>(Collections.singleton(requirement));
-	}
 }
diff --git a/...nox/src/org/eclipse/passage/lic/internal/equinox/requirements/RequirementsFromBundle.java b/...nox/src/org/eclipse/passage/lic/internal/equinox/requirements/RequirementsFromBundle.java
@@ -48,6 +48,7 @@ public String key() {
 	}
 
 	private final static class FromBundle {
+
 		private final Bundle bundle;
 
 		FromBundle(Bundle bundle) {

diff --git a/...x.tests/src/org/eclipse/passage/lic/internal/equinox/requirements/BundleManifestTest.java b/...x.tests/src/org/eclipse/passage/lic/internal/equinox/requirements/BundleManifestTest.java
@@ -22,6 +22,11 @@
 
 public final class BundleManifestTest {
 
+	@Test(expected = NullPointerException.class)
+	public void sourceIsMandatory() {
+		new BundleManifest(null);
+	}
+
 	@Test
 	public void readManifest() {
 		try {

diff --git a/...lipse/passage/lic/internal/equinox/requirements/ProvidedCapabilitiesFromManifestTest.java b/...lipse/passage/lic/internal/equinox/requirements/ProvidedCapabilitiesFromManifestTest.java
@@ -23,8 +23,22 @@
 
 public final class ProvidedCapabilitiesFromManifestTest {
 
+	@Test(expected = NullPointerException.class)
+	public void sourceIsMandatory() {
+		new ProvidedCapabilitiesFromManifest(null);
+	}
+
 	@Test
 	public void empty() {
+		try {
+			assertFalse(new ProvidedCapabilitiesFromManifest("").get().isPresent()); //$NON-NLS-1$
+		} catch (LicensingException e) {
+			fail("Manifest scanning is not supposed to fail on valid data"); //$NON-NLS-1$
+		}
+	}
+
+	@Test
+	public void noCapabilitiesDeclared() {
 		String manifest = "Manifest-Version: 1.0\r\n" //$NON-NLS-1$
 				+ "Automatic-Module-Name: org.eclipse.passage.lic.equinox.tests\r\n" //$NON-NLS-1$
 				+ "Bundle-ManifestVersion: 2\r\n" //$NON-NLS-1$
@@ -54,19 +68,37 @@ public void existingNixLineEndings() {
 		testExistingDeclarations("\n"); //$NON-NLS-1$
 	}
 
+	@Test
+	public void fromManifestFile() {
+		try {
+			Optional<String> declarations = new ProvidedCapabilitiesFromManifest(//
+					new BundleManifest(//
+							new DataBundle().bundle()//
+					).get()//
+			).get();
+			assertCapabilitiesAreOk(declarations);
+		} catch (LicensingException e) {
+			fail("Is not supposed to fail on valid data"); //$NON-NLS-1$
+		}
+
+	}
+
 	private void testExistingDeclarations(String ending) {
 		try {
-			Optional<String> declarations = new ProvidedCapabilitiesFromManifest(manifest(ending)).get();
-			assertTrue(declarations.isPresent());
-			assertTrue(declarations.get().startsWith("licensing.feature;")); //$NON-NLS-1$
-			assertTrue(declarations.get().endsWith("licensing.feature")); //$NON-NLS-1$
-			assertTrue(declarations.get().contains("\"Euler number\"")); //$NON-NLS-1$
-			assertTrue(declarations.get().contains(";level=\"error\"")); //$NON-NLS-1$
+			assertCapabilitiesAreOk(new ProvidedCapabilitiesFromManifest(manifest(ending)).get());
 		} catch (LicensingException e) {
 			fail("Manifest scanning is not supposed to fail on valid data"); //$NON-NLS-1$
 		}
 	}
 
+	private void assertCapabilitiesAreOk(Optional<String> declarations) {
+		assertTrue(declarations.isPresent());
+		assertTrue(declarations.get().startsWith("licensing.feature;")); //$NON-NLS-1$
+		assertTrue(declarations.get().endsWith("licensing.feature")); //$NON-NLS-1$
+		assertTrue(declarations.get().contains("\"Euler number\"")); //$NON-NLS-1$
+		assertTrue(declarations.get().contains(";level=\"error\"")); //$NON-NLS-1$
+	}
+
 	private String manifest(String ending) {
 		return "Manifest-Version: 1.0" + ending //$NON-NLS-1$
 				+ "Automatic-Module-Name: org.eclipse.passage.lic.equinox.tests.data.requirements" + ending //$NON-NLS-1$