You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by da...@apache.org on 2018/09/20 09:43:35 UTC
[sling-org-apache-sling-feature-analyser] branch master updated
(eaa137d -> 2223816)
This is an automated email from the ASF dual-hosted git repository.
davidb pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-analyser.git.
from eaa137d trivial: added license header to *.md files
new 3155199 SLING-7925 - Donate new AnalyzerTask which is able to validate APIs/Regions
new 2223816 SLING-7925 - Donate new AnalyzerTask which is able to validate APIs/Regions
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
pom.xml | 6 +
.../analyser/task/impl/CheckApiRegions.java | 232 +++++++++++++++++++++
...canner.java => ApiRegionsExtensionScanner.java} | 43 ++--
...apache.sling.feature.analyser.task.AnalyserTask | 2 +-
...ache.sling.feature.scanner.spi.ExtensionScanner | 2 +-
.../analyser/task/impl/CheckApiRegionsTest.java | 182 ++++++++++++++++
6 files changed, 438 insertions(+), 29 deletions(-)
create mode 100644 src/main/java/org/apache/sling/feature/analyser/task/impl/CheckApiRegions.java
copy src/main/java/org/apache/sling/feature/scanner/impl/{RepoInitScanner.java => ApiRegionsExtensionScanner.java} (51%)
create mode 100644 src/test/java/org/apache/sling/feature/analyser/task/impl/CheckApiRegionsTest.java
[sling-org-apache-sling-feature-analyser] 01/02: SLING-7925 -
Donate new AnalyzerTask which is able to validate APIs/Regions
Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
davidb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-analyser.git
commit 3155199fb3ee9e6d3aff4ea363394cfb61c81a38
Author: Simo Tripodi <st...@adobe.com>
AuthorDate: Sat Sep 15 10:35:59 2018 +0200
SLING-7925 - Donate new AnalyzerTask which is able to validate
APIs/Regions
initial checkin
---
.../analyser/task/impl/CheckApiRegions.java | 222 +++++++++++++++++++++
.../scanner/impl/ApiRegionsExtensionScanner.java | 56 ++++++
...apache.sling.feature.analyser.task.AnalyserTask | 2 +-
...ache.sling.feature.scanner.spi.ExtensionScanner | 2 +-
4 files changed, 280 insertions(+), 2 deletions(-)
diff --git a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckApiRegions.java b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckApiRegions.java
new file mode 100644
index 0000000..6ef2b89
--- /dev/null
+++ b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckApiRegions.java
@@ -0,0 +1,222 @@
+/*
+ * 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.analyser.task.impl;
+
+import java.io.StringReader;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Formatter;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.TreeSet;
+
+import javax.json.Json;
+import javax.json.stream.JsonParser;
+import javax.json.stream.JsonParser.Event;
+
+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.scanner.BundleDescriptor;
+import org.apache.sling.feature.scanner.FeatureDescriptor;
+import org.apache.sling.feature.scanner.PackageInfo;
+
+public class CheckApiRegions implements AnalyserTask {
+
+ private static final String API_REGIONS_KEY = "api-regions";
+
+ private static final String NAME_KEY = "name";
+
+ private static final String EXPORTS_KEY = "exports";
+
+ @Override
+ public String getId() {
+ return API_REGIONS_KEY;
+ }
+
+ @Override
+ public String getName() {
+ return "Api Regions analyser task";
+ }
+
+ @Override
+ public void execute(AnalyserTaskContext ctx) throws Exception {
+ Feature feature = ctx.getFeature();
+
+ // extract and check the api-regions
+
+ Extensions extensions = feature.getExtensions();
+ Extension apiRegionsExtension = extensions.getByName(API_REGIONS_KEY);
+ if (apiRegionsExtension == null) {
+ // no need to be analyzed
+ return;
+ }
+
+ String jsonRepresentation = apiRegionsExtension.getJSON();
+ if (jsonRepresentation == null || jsonRepresentation.isEmpty()) {
+ // no need to be analyzed
+ return;
+ }
+
+ // read the api-regions and create a Sieve data structure for checks
+
+ ApiRegions apiRegions = fromJson(jsonRepresentation);
+
+ // then, for each bundle, get the Export-Package and process the packages
+
+ FeatureDescriptor featureDescriptor = ctx.getFeatureDescriptor();
+ for (BundleDescriptor bundleDescriptor : featureDescriptor.getBundleDescriptors()) {
+ for (PackageInfo packageInfo : bundleDescriptor.getExportedPackages()) {
+ String exportedPackage = packageInfo.getName();
+ // use the Sieve technique: remove bundle exported packages from the api-regions
+ apiRegions.remove(exportedPackage);
+ }
+ }
+
+ // final evaluation: if the Sieve is not empty, not all declared packages are exported by bundles of the same feature
+ if (!apiRegions.isEmpty()) {
+ // track a single error for each region
+ for (String region : apiRegions.getRegions()) {
+ Set<String> apis = apiRegions.getApis(region);
+ if (!apis.isEmpty()) {
+ Formatter formatter = new Formatter();
+ formatter.format("Region '%s' defined in feature '%s' declares %s package%s which %s not exported by any bundle:%n",
+ region,
+ feature.getId(),
+ apis.size(),
+ getExtension(apis, "", "s"),
+ getExtension(apis, "is", "are"));
+ apis.forEach(api -> formatter.format(" * %s%n", api));
+
+ ctx.reportError(formatter.toString());
+
+ formatter.close();
+ }
+ }
+ }
+ }
+
+ // utility methods
+
+ private static <T> String getExtension(Collection<T> collection, String singular, String plural) {
+ return collection.size() > 1 ? plural : singular;
+ }
+
+ private static ApiRegions fromJson(String jsonRepresentation) {
+ ApiRegions apiRegions = new ApiRegions();
+
+ // pointers
+ Event event;
+ String region = null;
+ Collection<String> apis = null;
+
+ JsonParser parser = Json.createParser(new StringReader(jsonRepresentation));
+ while (parser.hasNext()) {
+ event = parser.next();
+ if (Event.KEY_NAME == event) {
+ switch (parser.getString()) {
+ case NAME_KEY:
+ parser.next();
+ region = parser.getString();
+ break;
+
+ case EXPORTS_KEY:
+ apis = new LinkedList<>();
+
+ // start array
+ parser.next();
+
+ while (parser.hasNext() && Event.VALUE_STRING == parser.next()) {
+ String api = parser.getString();
+ // skip comments
+ if ('#' != api.charAt(0)) {
+ apis.add(api);
+ }
+ }
+
+ break;
+
+ default:
+ break;
+ }
+ } else if (Event.END_OBJECT == event) {
+ if (region != null && apis != null) {
+ apiRegions.add(region, apis);
+ }
+
+ region = null;
+ apis = null;
+ }
+ }
+
+ return apiRegions;
+ }
+
+ // the Sieve data structure to check exported packages
+
+ private static final class ApiRegions {
+
+ // class members
+
+ private final Map<String, Set<String>> apis = new TreeMap<>();
+
+ // ctor
+
+ protected ApiRegions() {
+ // it should not be directly instantiated outside this package
+ }
+
+ // methods
+
+ public void add(String region, Collection<String> exportedApis) {
+ apis.computeIfAbsent(region, k -> new TreeSet<>()).addAll(exportedApis);
+ }
+
+ public Iterable<String> getRegions() {
+ return apis.keySet();
+ }
+
+ public Set<String> getApis(String region) {
+ return apis.computeIfAbsent(region, k -> Collections.emptySet());
+ }
+
+ public void remove(String packageName) {
+ apis.values().forEach(apis -> apis.remove(packageName));
+ }
+
+ public boolean isEmpty() {
+ for (Set<String> packages : apis.values()) {
+ if (!packages.isEmpty()) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ @Override
+ public String toString() {
+ return apis.toString().replace(',', '\n');
+ }
+
+ }
+
+
+}
diff --git a/src/main/java/org/apache/sling/feature/scanner/impl/ApiRegionsExtensionScanner.java b/src/main/java/org/apache/sling/feature/scanner/impl/ApiRegionsExtensionScanner.java
new file mode 100644
index 0000000..0aaa9b5
--- /dev/null
+++ b/src/main/java/org/apache/sling/feature/scanner/impl/ApiRegionsExtensionScanner.java
@@ -0,0 +1,56 @@
+/*
+ * 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.scanner.impl;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.apache.sling.feature.Artifact;
+import org.apache.sling.feature.Extension;
+import org.apache.sling.feature.Feature;
+import org.apache.sling.feature.io.ArtifactManager;
+import org.apache.sling.feature.scanner.BundleDescriptor;
+import org.apache.sling.feature.scanner.ContainerDescriptor;
+import org.apache.sling.feature.scanner.FeatureDescriptor;
+import org.apache.sling.feature.scanner.spi.ExtensionScanner;
+
+public class ApiRegionsExtensionScanner implements ExtensionScanner {
+
+ @Override
+ public String getId() {
+ return "api-regions";
+ }
+
+ @Override
+ public String getName() {
+ return "Api Regions extention scanner";
+ }
+
+ @Override
+ public ContainerDescriptor scan(Feature feature, Extension extension, ArtifactManager manager) throws IOException {
+ FeatureDescriptor featureDescriptor = new FeatureDescriptorImpl(feature);
+
+ for (Artifact artifact : feature.getBundles()) {
+ File file = manager.getArtifactHandler(artifact.getId().toMvnUrl()).getFile();
+ BundleDescriptor bundleDescriptor = new BundleDescriptorImpl(artifact, file, artifact.getStartOrder());
+ featureDescriptor.getBundleDescriptors().add(bundleDescriptor);
+ }
+
+ return featureDescriptor;
+ }
+
+}
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 dabd8fb..7f58e8d 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
@@ -2,4 +2,4 @@ org.apache.sling.feature.analyser.task.impl.CheckBundleExportsImports
org.apache.sling.feature.analyser.task.impl.CheckBundlesForInitialContent
org.apache.sling.feature.analyser.task.impl.CheckBundlesForResources
org.apache.sling.feature.analyser.task.impl.CheckRequirementsCapabilities
-
+org.apache.sling.feature.analyser.task.impl.CheckApiRegions
diff --git a/src/main/resources/META-INF/services/org.apache.sling.feature.scanner.spi.ExtensionScanner b/src/main/resources/META-INF/services/org.apache.sling.feature.scanner.spi.ExtensionScanner
index 643c7cf..528834d 100644
--- a/src/main/resources/META-INF/services/org.apache.sling.feature.scanner.spi.ExtensionScanner
+++ b/src/main/resources/META-INF/services/org.apache.sling.feature.scanner.spi.ExtensionScanner
@@ -1,3 +1,3 @@
org.apache.sling.feature.scanner.impl.ContentPackagesExtensionScanner
org.apache.sling.feature.scanner.impl.RepoInitScanner
-
+org.apache.sling.feature.scanner.impl.ApiRegionsExtensionScanner
[sling-org-apache-sling-feature-analyser] 02/02: SLING-7925 -
Donate new AnalyzerTask which is able to validate APIs/Regions
Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
davidb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-analyser.git
commit 2223816471b7d0ed97f4931d50fea5c5c9c41bbb
Author: Simo Tripodi <st...@adobe.com>
AuthorDate: Wed Sep 19 16:36:50 2018 +0200
SLING-7925 - Donate new AnalyzerTask which is able to validate
APIs/Regions
* added unit tests
* added guard against not valid/expected JSON
---
pom.xml | 6 +
.../analyser/task/impl/CheckApiRegions.java | 12 +-
.../analyser/task/impl/CheckApiRegionsTest.java | 182 +++++++++++++++++++++
3 files changed, 199 insertions(+), 1 deletion(-)
diff --git a/pom.xml b/pom.xml
index 6511b76..478ea2a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -164,5 +164,11 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <version>2.22.0</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
diff --git a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckApiRegions.java b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckApiRegions.java
index 6ef2b89..cb0502f 100644
--- a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckApiRegions.java
+++ b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckApiRegions.java
@@ -29,6 +29,7 @@ import java.util.TreeSet;
import javax.json.Json;
import javax.json.stream.JsonParser;
import javax.json.stream.JsonParser.Event;
+import javax.json.stream.JsonParsingException;
import org.apache.sling.feature.Extension;
import org.apache.sling.feature.Extensions;
@@ -78,7 +79,16 @@ public class CheckApiRegions implements AnalyserTask {
// read the api-regions and create a Sieve data structure for checks
- ApiRegions apiRegions = fromJson(jsonRepresentation);
+ ApiRegions apiRegions;
+ try {
+ apiRegions = fromJson(jsonRepresentation);
+ } catch (JsonParsingException e) {
+ ctx.reportError("API Regions '"
+ + jsonRepresentation
+ + "' does not represent a valid JSON 'api-regions': "
+ + e.getMessage());
+ return;
+ }
// then, for each bundle, get the Export-Package and process the packages
diff --git a/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckApiRegionsTest.java b/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckApiRegionsTest.java
new file mode 100644
index 0000000..bfbf446
--- /dev/null
+++ b/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckApiRegionsTest.java
@@ -0,0 +1,182 @@
+/*
+ * 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.analyser.task.impl;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.File;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.jar.Manifest;
+
+import org.apache.sling.feature.Artifact;
+import org.apache.sling.feature.ArtifactId;
+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.scanner.BundleDescriptor;
+import org.apache.sling.feature.scanner.FeatureDescriptor;
+import org.apache.sling.feature.scanner.PackageInfo;
+import org.junit.Test;
+
+public class CheckApiRegionsTest {
+
+ private AnalyserTask checkApiRegions = new CheckApiRegions();
+
+ @Test
+ public void testNoApiRegionsExtension() throws Exception {
+ List<String> errors = execute((Extension) null);
+ assertTrue(errors.isEmpty());
+ }
+
+ @Test
+ public void testNullApiRegionsJSON() throws Exception {
+ List<String> errors = execute((String) null);
+ assertTrue(errors.isEmpty());
+ }
+
+ @Test
+ public void testEmptyApiRegionsJSON() throws Exception {
+ List<String> errors = execute("");
+ 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'"));
+ }
+
+ @Test
+ public void testNotValidApiRegionJson() throws Exception {
+ List<String> errors = execute("[{\"name\": \"global\",\"exports\": [\"org.osgi.util.function.doesnotexist\"]}]");
+
+ assertFalse(errors.isEmpty());
+ assertTrue(errors.iterator()
+ .next()
+ .startsWith("Region 'global' defined in feature 'org.apache.sling.testing:org.apache.sling.testing.apiregions:1.0.0' declares 1 package which is not exported by any bundle"));
+ }
+
+ @Test
+ public void testValidApiRegionJson() throws Exception {
+ List<String> errors = execute("[{\"name\": \"global\",\"exports\": [\"org.osgi.util.function\"]}]");
+ assertTrue(errors.isEmpty());
+ }
+
+ private List<String> execute(String apiRegionJSON) throws Exception {
+ Extension extension = mock(Extension.class);
+ when(extension.getJSON()).thenReturn(apiRegionJSON);
+
+ return execute(extension);
+ }
+
+ private List<String> execute(Extension extension) throws Exception {
+ Extensions extensions = mock(Extensions.class);
+ when(extensions.getByName("api-regions")).thenReturn(extension);
+
+ Feature feature = mock(Feature.class);
+ when(feature.getId()).thenReturn(new ArtifactId("org.apache.sling.testing",
+ "org.apache.sling.testing.apiregions",
+ "1.0.0",
+ null,
+ null));
+ when(feature.getExtensions()).thenReturn(extensions);
+
+ AnalyserTaskContext ctx = mock(AnalyserTaskContext.class);
+ when(ctx.getFeature()).thenReturn(feature);
+
+ PackageInfo packageInfo = new PackageInfo("org.osgi.util.function", "1.0", false);
+
+ BundleDescriptor bundleDescriptor = new TestBundleDescriptor();
+ bundleDescriptor.getExportedPackages().add(packageInfo);
+
+ FeatureDescriptor featureDescriptor = new TestFeatureDescriptor();
+ featureDescriptor.getBundleDescriptors().add(bundleDescriptor);
+
+ when(ctx.getFeatureDescriptor()).thenReturn(featureDescriptor);
+
+ List<String> errors = new LinkedList<>();
+ doAnswer(invocation -> {
+ String error = invocation.getArgument(0);
+ errors.add(error);
+ return null;
+ }).when(ctx).reportError(anyString());
+
+ checkApiRegions.execute(ctx);
+
+ return errors;
+ }
+
+ private static final class TestFeatureDescriptor extends FeatureDescriptor {
+
+ @Override
+ public Feature getFeature() {
+ return null;
+ }
+
+ }
+
+ private static final class TestBundleDescriptor extends BundleDescriptor {
+
+ @Override
+ public File getArtifactFile() {
+ // do nothing
+ return null;
+ }
+
+ @Override
+ public Artifact getArtifact() {
+ // do nothing
+ return null;
+ }
+
+ @Override
+ public Manifest getManifest() {
+ // do nothing
+ return null;
+ }
+
+ @Override
+ public String getBundleVersion() {
+ // do nothing
+ return null;
+ }
+
+ @Override
+ public String getBundleSymbolicName() {
+ // do nothing
+ return null;
+ }
+
+ @Override
+ public int getBundleStartLevel() {
+ // do nothing
+ return 0;
+ }
+
+ }
+
+}