SLING-11068 split up analyser task for unversioned packages - move analyser to analyser module
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTask.java b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTask.java
index adb244f..1b5ebce 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTask.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTask.java
@@ -16,12 +16,6 @@
*/
package org.apache.sling.feature.extension.apiregions.analyser;
-import java.io.IOException;
-import javax.json.JsonArray;
-
-import org.apache.sling.feature.Extension;
-import org.apache.sling.feature.Extensions;
-import org.apache.sling.feature.Feature;
import org.apache.sling.feature.analyser.task.AnalyserTask;
import org.apache.sling.feature.analyser.task.AnalyserTaskContext;
import org.apache.sling.feature.extension.apiregions.api.ApiRegions;
@@ -30,41 +24,19 @@
@Override
public final void execute(AnalyserTaskContext ctx) throws Exception {
- Feature feature = ctx.getFeature();
-
- // extract and check the api-regions
-
- Extensions extensions = feature.getExtensions();
- Extension apiRegionsExtension = extensions.getByName(ApiRegions.EXTENSION_NAME);
- if (apiRegionsExtension == null) {
- // no need to be analyzed
- return;
- }
-
- if (apiRegionsExtension.getJSON() == null || apiRegionsExtension.getJSON().isEmpty()) {
- // no need to be analyzed
- return;
- }
-
- if (apiRegionsExtension.getJSONStructure() == null) {
- ctx.reportError("API Regions '" + apiRegionsExtension.getJSON()
- + "' does not represent a valid JSON 'api-regions'");
- return;
- }
-
- // read the api-regions and create a Sieve data structure for checks
-
+ // read the api-regions
ApiRegions apiRegions;
try {
- apiRegions = ApiRegions.parse((JsonArray) apiRegionsExtension.getJSONStructure());
- } catch (IOException e) {
- ctx.reportError("API Regions '"
- + apiRegionsExtension.getJSON()
- + "' does not represent a valid JSON 'api-regions': "
+ apiRegions = ApiRegions.getApiRegions(ctx.getFeature());
+ } catch (final IllegalArgumentException e) {
+ ctx.reportError("API Regions does not represent a valid JSON 'api-regions': "
+ e.getMessage());
return;
}
-
+ if ( apiRegions == null ) {
+ // no need to be analysed
+ return;
+ }
execute(apiRegions, ctx);
}
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleUnversionedPackages.java b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleUnversionedPackages.java
deleted file mode 100644
index cda708d..0000000
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleUnversionedPackages.java
+++ /dev/null
@@ -1,107 +0,0 @@
-/*
- * 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.sling.feature.extension.apiregions.analyser;
-
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-
-import org.apache.sling.feature.analyser.task.AnalyserTask;
-import org.apache.sling.feature.analyser.task.AnalyserTaskContext;
-import org.apache.sling.feature.extension.apiregions.api.ApiRegions;
-import org.apache.sling.feature.scanner.BundleDescriptor;
-import org.apache.sling.feature.scanner.PackageInfo;
-import org.osgi.framework.Version;
-
-
-public class CheckApiRegionsBundleUnversionedPackages implements AnalyserTask {
-
- /** Ignore JDK packages */
- private static final List<String> IGNORED_IMPORT_PREFIXES = Arrays.asList("java.", "javax.", "org.w3c.", "org.xml.");
-
- @Override
- public String getName() {
- return "Bundle Unversioned Packages Check";
- }
-
- @Override
- public String getId() {
- return ApiRegions.EXTENSION_NAME + "-unversioned-packages";
- }
-
- private boolean ignoreImportPackage(final String name) {
- for(final String prefix : IGNORED_IMPORT_PREFIXES) {
- if ( name.startsWith(prefix) ) {
- return true;
- }
- }
- return false;
- }
-
- @Override
- public void execute(final AnalyserTaskContext ctx) throws Exception {
- for(final BundleDescriptor info : ctx.getFeatureDescriptor().getBundleDescriptors()) {
-
-
- final List<PackageInfo> exportWithoutVersion = new ArrayList<>();
- for(final PackageInfo i : info.getExportedPackages()) {
- if ( i.getPackageVersion().compareTo(Version.emptyVersion) == 0 ) {
- exportWithoutVersion.add(i);
- }
- }
- final List<PackageInfo> importWithoutVersion = new ArrayList<>();
- for(final PackageInfo i : info.getImportedPackages()) {
- if ( i.getVersion() == null && !ignoreImportPackage(i.getName()) ) {
- importWithoutVersion.add(i);
- }
- }
-
- final String key = "Bundle ".concat(info.getArtifact().getId().getArtifactId())
- .concat(":").concat(info.getArtifact().getId().getVersion());
-
- if ( !importWithoutVersion.isEmpty() ) {
- ctx.reportArtifactWarning(info.getArtifact().getId(),
- key.concat(" is importing ").concat(getPackageInfo(importWithoutVersion)).concat(" without specifying a version range."));
- }
- if ( !exportWithoutVersion.isEmpty() ) {
- ctx.reportArtifactWarning(info.getArtifact().getId(),
- key.concat(" is exporting ").concat(getPackageInfo(exportWithoutVersion)).concat(" without a version."));
- }
- }
- }
-
- private String getPackageInfo(final List<PackageInfo> pcks) {
- if ( pcks.size() == 1 ) {
- return "package ".concat(pcks.get(0).getName());
- }
- final StringBuilder sb = new StringBuilder("packages ");
- boolean first = true;
- sb.append('[');
- for(final PackageInfo info : pcks) {
- if ( first ) {
- first = false;
- } else {
- sb.append(", ");
- }
- sb.append(info.getName());
- }
- sb.append(']');
- return sb.toString();
- }
-}
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsOrder.java b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsOrder.java
index bbfff43..feb89bf 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsOrder.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsOrder.java
@@ -16,20 +16,14 @@
*/
package org.apache.sling.feature.extension.apiregions.analyser;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
-import javax.json.JsonArray;
-import org.apache.sling.feature.Extension;
-import org.apache.sling.feature.Extensions;
-import org.apache.sling.feature.Feature;
-import org.apache.sling.feature.analyser.task.AnalyserTask;
import org.apache.sling.feature.analyser.task.AnalyserTaskContext;
import org.apache.sling.feature.extension.apiregions.api.ApiRegion;
import org.apache.sling.feature.extension.apiregions.api.ApiRegions;
-public class CheckApiRegionsOrder implements AnalyserTask {
+public class CheckApiRegionsOrder extends AbstractApiRegionsAnalyserTask {
@Override
public String getId() {
@@ -42,21 +36,15 @@
}
@Override
- public final void execute(AnalyserTaskContext ctx) throws Exception {
- String order = ctx.getConfiguration().get("order");
- Feature feature = ctx.getFeature();
- if (feature == null) {
- reportError(ctx, "No feature found. Illegal Analyser State.");
- return;
- }
-
+ public final void execute(final ApiRegions apiRegions, final AnalyserTaskContext ctx) throws Exception {
+ final String order = ctx.getConfiguration().get("order");
if (order == null) {
- reportError(ctx, "This analyser task must be configured: " + getId() + " for feature " + feature.getId());
+ reportError(ctx, "This analyser task must be configured: " + getId() + " for feature " + ctx.getFeature().getId());
reportError(ctx, "Must specify configuration key 'order'.");
return;
}
- String[] sl = order.split("[,]");
+ final String[] sl = order.split("[,]");
List<String> prescribedOrder = new ArrayList<>();
for (String s : sl) {
s = s.trim();
@@ -68,39 +56,20 @@
return;
}
- // extract and check the api-regions
- Extensions extensions = feature.getExtensions();
- Extension apiRegionsExtension = extensions.getByName(ApiRegions.EXTENSION_NAME);
- if (apiRegionsExtension == null) {
- // no need to be analyzed
- return;
- }
-
- String jsonRepresentation = apiRegionsExtension.getJSON();
- if (jsonRepresentation == null || jsonRepresentation.isEmpty()) {
- // no need to be analyzed
- return;
- }
-
- try {
- int regionIdx = 0;
- ApiRegions apiRegions = ApiRegions.parse((JsonArray) apiRegionsExtension.getJSONStructure());
- for (final ApiRegion region : apiRegions.listRegions()) {
- String name = region.getName();
- if (!prescribedOrder.contains(name)) {
- reportError(ctx, "Region found with undeclared name: " + name);
- return;
- }
- int prevIdx = regionIdx;
- regionIdx = validateRegion(regionIdx, prescribedOrder, name);
- if (regionIdx < 0) {
- reportError(ctx, "Region '" + name + "' appears in the wrong order. It appears after '"
- + prescribedOrder.get(prevIdx) + "'. Order of regions should be " + prescribedOrder);
- return;
- }
+ int regionIdx = 0;
+ for (final ApiRegion region : apiRegions.listRegions()) {
+ String name = region.getName();
+ if (!prescribedOrder.contains(name)) {
+ reportError(ctx, "Region found with undeclared name: " + name);
+ return;
}
- } catch (final IOException e) {
- ctx.reportError("Invalid api regions");
+ int prevIdx = regionIdx;
+ regionIdx = validateRegion(regionIdx, prescribedOrder, name);
+ if (regionIdx < 0) {
+ reportError(ctx, "Region '" + name + "' appears in the wrong order. It appears after '"
+ + prescribedOrder.get(prevIdx) + "'. Order of regions should be " + prescribedOrder);
+ return;
+ }
}
}
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegions.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegions.java
index c40b98c..618a4bb 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegions.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegions.java
@@ -314,6 +314,9 @@
if ( ext.getType() != ExtensionType.JSON ) {
throw new IllegalArgumentException("Extension " + ext.getName() + " must have JSON type");
}
+ if ( ext.getJSONStructure() == null ) {
+ return new ApiRegions();
+ }
try {
return parse(ext.getJSONStructure().asJsonArray());
} catch ( final IOException ioe) {
diff --git a/src/main/resources/META-INF/services/org.apache.sling.feature.analyser.task.AnalyserTask b/src/main/resources/META-INF/services/org.apache.sling.feature.analyser.task.AnalyserTask
index 4f80111..1ed0442 100644
--- a/src/main/resources/META-INF/services/org.apache.sling.feature.analyser.task.AnalyserTask
+++ b/src/main/resources/META-INF/services/org.apache.sling.feature.analyser.task.AnalyserTask
@@ -3,7 +3,6 @@
org.apache.sling.feature.extension.apiregions.analyser.CheckApiRegionsDuplicates
org.apache.sling.feature.extension.apiregions.analyser.CheckApiRegionsOrder
org.apache.sling.feature.extension.apiregions.analyser.CheckApiRegionsBundleExportsImports
-org.apache.sling.feature.extension.apiregions.analyser.CheckApiRegionsBundleUnversionedPackages
org.apache.sling.feature.extension.apiregions.analyser.CheckApiRegionsCrossFeatureDups
org.apache.sling.feature.extension.apiregions.analyser.CheckArtifactRules
org.apache.sling.feature.extension.apiregions.analyser.CheckConfigurationApi
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTaskTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTaskTest.java
index ba362d0..c6c1484 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTaskTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTaskTest.java
@@ -32,10 +32,12 @@
import org.apache.sling.feature.Artifact;
import org.apache.sling.feature.ArtifactId;
import org.apache.sling.feature.Extension;
+import org.apache.sling.feature.ExtensionType;
import org.apache.sling.feature.Extensions;
import org.apache.sling.feature.Feature;
import org.apache.sling.feature.analyser.task.AnalyserTask;
import org.apache.sling.feature.analyser.task.AnalyserTaskContext;
+import org.apache.sling.feature.extension.apiregions.api.ApiRegions;
import org.apache.sling.feature.scanner.BundleDescriptor;
import org.apache.sling.feature.scanner.FeatureDescriptor;
import org.apache.sling.feature.scanner.PackageInfo;
@@ -89,22 +91,17 @@
assertTrue(errors.isEmpty());
}
- @Test
- public void testApiRegionsIsNotJSON() throws Exception {
- List<String> errors = execute("this is not a JSON string");
-
- assertFalse(errors.isEmpty());
- assertTrue(errors.iterator().next().contains("does not represent a valid JSON 'api-regions'"));
- }
-
protected final List<String> execute(String apiRegionJSON) throws Exception {
Extension extension = mock(Extension.class);
+ when(extension.getName()).thenReturn(ApiRegions.EXTENSION_NAME);
when(extension.getJSON()).thenReturn(apiRegionJSON);
+ when(extension.getType()).thenReturn(ExtensionType.JSON);
if (apiRegionJSON != null) {
JsonReader reader = Json.createReader(new StringReader(apiRegionJSON));
JsonArray array = null;
try {
array = reader.readArray();
+ when(extension.getType()).thenReturn(ExtensionType.JSON);
} catch (final JsonParsingException | NothingToRead ignore) {
// ignored
}
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleUnversionedPackagesTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleUnversionedPackagesTest.java
deleted file mode 100644
index ff4695c..0000000
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleUnversionedPackagesTest.java
+++ /dev/null
@@ -1,84 +0,0 @@
-/*
- * 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.sling.feature.extension.apiregions.analyser;
-
-import org.apache.sling.feature.*;
-import org.apache.sling.feature.analyser.task.AnalyserTaskContext;
-import org.apache.sling.feature.extension.apiregions.api.ApiRegions;
-import org.apache.sling.feature.scanner.BundleDescriptor;
-import org.apache.sling.feature.scanner.FeatureDescriptor;
-import org.apache.sling.feature.scanner.impl.BundleDescriptorImpl;
-import org.apache.sling.feature.scanner.impl.FeatureDescriptorImpl;
-import org.junit.BeforeClass;
-import org.junit.Test;
-import org.mockito.Mockito;
-
-import java.io.File;
-import java.io.IOException;
-import static org.junit.Assert.assertEquals;
-
-public class CheckApiRegionsBundleUnversionedPackagesTest {
- private static File resourceRoot;
-
- @BeforeClass
- public static void setupClass() {
- resourceRoot =
- new File(CheckApiRegionsBundleUnversionedPackagesTest.class.
- getResource("/test-framework.jar").getFile()).getParentFile();
- }
-
- @Test
- public void testId() {
- assertEquals(ApiRegions.EXTENSION_NAME + "-unversioned-packages", new CheckApiRegionsBundleUnversionedPackages().getId());
- }
-
- @Test
- public void testBundleUnversionedPackage() throws Exception {
- CheckApiRegionsBundleUnversionedPackages t = new CheckApiRegionsBundleUnversionedPackages();
-
- Feature f = new Feature(ArtifactId.fromMvnId("f:f:1"));
- f.setComplete(true);
- FeatureDescriptor fd = new FeatureDescriptorImpl(f);
-
- fdAddBundle(fd, "g:b1:1", "test-bundle5.jar");
-
- AnalyserTaskContext ctx = Mockito.mock(AnalyserTaskContext.class);
- Mockito.when(ctx.getFeature()).thenReturn(f);
- Mockito.when(ctx.getFeatureDescriptor()).thenReturn(fd);
- t.execute(ctx);
-
- Mockito.verify(ctx, Mockito.times(2)).reportArtifactWarning(Mockito.any(), Mockito.anyString());
-
- Mockito.verify(ctx).reportArtifactWarning(
- Mockito.eq(ArtifactId.fromMvnId("g:b1:1")),
- Mockito.contains("org.foo.i"));
- Mockito.verify(ctx).reportArtifactWarning(
- Mockito.eq(ArtifactId.fromMvnId("g:b1:1")),
- Mockito.contains("org.foo.e"));
- }
-
-
- private void fdAddBundle(FeatureDescriptor fd, String id, String file, ArtifactId... origins) throws IOException {
- Artifact artifact = new Artifact(ArtifactId.fromMvnId(id));
- artifact.setFeatureOrigins(origins);
- BundleDescriptor bd1 = new BundleDescriptorImpl(
- artifact, new File(resourceRoot, file).toURI().toURL(), 0);
- fd.getBundleDescriptors().add(bd1);
- }
-}
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsOrderTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsOrderTest.java
index 3948129..ca59764 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsOrderTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsOrderTest.java
@@ -77,7 +77,7 @@
Mockito.when(ctx.getFeature()).thenReturn(f);
caro.execute(ctx);
- Mockito.verify(ctx).reportError(Mockito.contains("Invalid api regions"));
+ Mockito.verify(ctx).reportError(Mockito.contains("API Regions does not represent a valid JSON"));
}
@Test
@@ -118,6 +118,12 @@
@Test
public void testEmptyOrder() throws Exception {
+ Extension e = new Extension(ExtensionType.JSON, "api-regions", ExtensionState.OPTIONAL);
+ e.setJSON("[{\"name\":\"foo\"}]");
+
+ Feature f = new Feature(ArtifactId.fromMvnId("a:b:1"));
+ f.getExtensions().add(e);
+
CheckApiRegionsOrder caro = new CheckApiRegionsOrder();
Map<String, String> cfgMap = new HashMap<>();
@@ -125,30 +131,25 @@
AnalyserTaskContext ctx = Mockito.mock(AnalyserTaskContext.class);
Mockito.when(ctx.getConfiguration()).thenReturn(cfgMap);
- Mockito.when(ctx.getFeature()).thenReturn(new Feature(ArtifactId.fromMvnId("a:b:1")));
+ Mockito.when(ctx.getFeature()).thenReturn(f);
caro.execute(ctx);
Mockito.verify(ctx).reportError(Mockito.contains("No regions"));
}
@Test
- public void testNoFeature() throws Exception {
- CheckApiRegionsOrder caro = new CheckApiRegionsOrder();
-
- AnalyserTaskContext ctx = Mockito.mock(AnalyserTaskContext.class);
- Mockito.when(ctx.getConfiguration()).thenReturn(new HashMap<>());
-
- caro.execute(ctx);
- Mockito.verify(ctx).reportError(Mockito.contains("No feature"));
- }
-
- @Test
public void testNoOrderConfig() throws Exception {
+ Extension e = new Extension(ExtensionType.JSON, "api-regions", ExtensionState.OPTIONAL);
+ e.setJSON("[{\"name\":\"foo\"}]");
+
+ Feature f = new Feature(ArtifactId.fromMvnId("a:b:1"));
+ f.getExtensions().add(e);
+
CheckApiRegionsOrder caro = new CheckApiRegionsOrder();
AnalyserTaskContext ctx = Mockito.mock(AnalyserTaskContext.class);
Mockito.when(ctx.getConfiguration()).thenReturn(new HashMap<>());
- Mockito.when(ctx.getFeature()).thenReturn(new Feature(ArtifactId.fromMvnId("a:b:1")));
+ Mockito.when(ctx.getFeature()).thenReturn(f);
caro.execute(ctx);
Mockito.verify(ctx).reportError(Mockito.contains("'order'"));
