You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2022/01/18 08:37:49 UTC

[sling-org-apache-sling-feature-analyser] branch master updated: SLING-11068 split up analyser task for unversioned packages - move analyser to analyser module

This is an automated email from the ASF dual-hosted git repository.

cziegeler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-analyser.git


The following commit(s) were added to refs/heads/master by this push:
     new 6c29041  SLING-11068 split up analyser task for unversioned packages - move analyser to analyser module
6c29041 is described below

commit 6c29041bb06d793da848a533a42b83d01d34e030
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Tue Jan 18 09:34:57 2022 +0100

    SLING-11068 split up analyser task for unversioned packages - move analyser to analyser module
---
 pom.xml                                            |   2 +-
 .../task/impl/CheckBundleExportsImports.java       | 107 +++++----------------
 .../task/impl/CheckBundleUnversionedPackages.java  | 106 ++++++++++++++++++++
 ...apache.sling.feature.analyser.task.AnalyserTask |   1 +
 .../impl/CheckBundleUnversionedPackagesTest.java   |  87 +++++++++++++++++
 src/test/resources/test-bundle-unversioned.jar     | Bin 0 -> 458 bytes
 6 files changed, 217 insertions(+), 86 deletions(-)

diff --git a/pom.xml b/pom.xml
index a742b45..2af6caa 100644
--- a/pom.xml
+++ b/pom.xml
@@ -22,7 +22,7 @@
     </parent>
 
     <artifactId>org.apache.sling.feature.analyser</artifactId>
-    <version>1.5.3-SNAPSHOT</version>
+    <version>1.6.0-SNAPSHOT</version>
 
     <name>Apache Sling Feature Model Analyser</name>
     <description>
diff --git a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImports.java b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImports.java
index 05b5de3..7f22ded 100644
--- a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImports.java
+++ b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImports.java
@@ -20,7 +20,6 @@ package org.apache.sling.feature.analyser.task.impl;
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -31,13 +30,9 @@ 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.PackageInfo;
-import org.osgi.framework.Version;
 
 public class CheckBundleExportsImports 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 Import/Export Check";
@@ -50,12 +45,8 @@ public class CheckBundleExportsImports implements AnalyserTask {
 
     public static final class Report {
 
-        public List<PackageInfo> exportWithoutVersion = new ArrayList<>();
-
         public List<PackageInfo> exportMatchingSeveral = new ArrayList<>();
 
-        public List<PackageInfo> importWithoutVersion = new ArrayList<>();
-
         public List<PackageInfo> missingExports = new ArrayList<>();
 
         public List<PackageInfo> missingExportsWithVersion = new ArrayList<>();
@@ -64,62 +55,17 @@ public class CheckBundleExportsImports implements AnalyserTask {
     }
 
     private Report getReport(final Map<BundleDescriptor, Report> reports, final BundleDescriptor info) {
-        Report report = reports.get(info);
-        if ( report == null ) {
-            report = new Report();
-            reports.put(info, report);
-        }
-        return report;
-    }
-
-    private void checkForVersionOnExportedPackages(final AnalyserTaskContext ctx, final Map<BundleDescriptor, Report> reports) {
-        for(final BundleDescriptor info : ctx.getFeatureDescriptor().getBundleDescriptors()) {
-            if ( info.getExportedPackages() != null ) {
-                for(final PackageInfo i : info.getExportedPackages()) {
-                    if ( i.getPackageVersion().compareTo(Version.emptyVersion) == 0 ) {
-                        getReport(reports, info).exportWithoutVersion.add(i);
-                    }
-                }
-            }
-        }
-    }
-
-    private boolean ignoreImportPackage(final String name) {
-        for(final String prefix : IGNORED_IMPORT_PREFIXES) {
-            if ( name.startsWith(prefix) ) {
-                return true;
-            }
-        }
-        return false;
-    }
-
-    private void checkForVersionOnImportingPackages(final AnalyserTaskContext ctx, final Map<BundleDescriptor, Report> reports) {
-        for(final BundleDescriptor info : ctx.getFeatureDescriptor().getBundleDescriptors()) {
-            if ( info.getImportedPackages() != null ) {
-                for(final PackageInfo i : info.getImportedPackages()) {
-                    if ( i.getVersion() == null && !ignoreImportPackage(i.getName()) ) {
-                        getReport(reports, info).importWithoutVersion.add(i);
-                    }
-                }
-            }
-        }
+        return reports.computeIfAbsent(info, key -> new Report());
     }
 
-
     @Override
     public void execute(final AnalyserTaskContext ctx) throws IOException {
         // basic checks
         final Map<BundleDescriptor, Report> reports = new HashMap<>();
-        checkForVersionOnExportedPackages(ctx, reports);
-        checkForVersionOnImportingPackages(ctx, reports);
 
         final SortedMap<Integer, List<BundleDescriptor>> bundlesMap = new TreeMap<>();
         for(final BundleDescriptor bi : ctx.getFeatureDescriptor().getBundleDescriptors()) {
-            List<BundleDescriptor> list = bundlesMap.get(bi.getArtifact().getStartOrder());
-            if ( list == null ) {
-                list = new ArrayList<>();
-                bundlesMap.put(bi.getArtifact().getStartOrder(), list);
-            }
+            List<BundleDescriptor> list = bundlesMap.computeIfAbsent(bi.getArtifact().getStartOrder(), key -> new ArrayList<>());
             list.add(bi);
         }
 
@@ -132,37 +78,35 @@ public class CheckBundleExportsImports implements AnalyserTask {
         for(final Map.Entry<Integer, List<BundleDescriptor>> entry : bundlesMap.entrySet()) {
             // first add all exporting bundles
             for(final BundleDescriptor info : entry.getValue()) {
-                if ( info.getExportedPackages() != null ) {
+                if ( !info.getExportedPackages().isEmpty() ) {
                     exportingBundles.add(info);
                 }
             }
             // check importing bundles
             for(final BundleDescriptor info : entry.getValue()) {
-                if ( info.getImportedPackages() != null ) {
-                    for(final PackageInfo pck : info.getImportedPackages() ) {
-                        final List<BundleDescriptor> candidates = getCandidates(exportingBundles, pck);
-                        if ( candidates.isEmpty() ) {
+                for(final PackageInfo pck : info.getImportedPackages() ) {
+                    final List<BundleDescriptor> candidates = getCandidates(exportingBundles, pck);
+                    if ( candidates.isEmpty() ) {
+                        if ( pck.isOptional() ) {
+                            getReport(reports, info).missingExportsForOptional.add(pck);
+                        } else {
+                            getReport(reports, info).missingExports.add(pck);
+                        }
+                    } else {
+                        final List<BundleDescriptor> matchingCandidates = new ArrayList<>();
+                        for (final BundleDescriptor i : candidates) {
+                            if (i.isExportingPackage(pck)) {
+                                matchingCandidates.add(i);
+                            }
+                        }
+                        if ( matchingCandidates.isEmpty() ) {
                             if ( pck.isOptional() ) {
                                 getReport(reports, info).missingExportsForOptional.add(pck);
                             } else {
-                                getReport(reports, info).missingExports.add(pck);
-                            }
-                        } else {
-                            final List<BundleDescriptor> matchingCandidates = new ArrayList<>();
-                            for (final BundleDescriptor i : candidates) {
-                                if (i.isExportingPackage(pck)) {
-                                    matchingCandidates.add(i);
-                                }
-                            }
-                            if ( matchingCandidates.isEmpty() ) {
-                                if ( pck.isOptional() ) {
-                                    getReport(reports, info).missingExportsForOptional.add(pck);
-                                } else {
-                                    getReport(reports, info).missingExportsWithVersion.add(pck);
-                                }
-                            } else if ( matchingCandidates.size() > 1 ) {
-                                getReport(reports, info).exportMatchingSeveral.add(pck);
+                                getReport(reports, info).missingExportsWithVersion.add(pck);
                             }
+                        } else if ( matchingCandidates.size() > 1 ) {
+                            getReport(reports, info).exportMatchingSeveral.add(pck);
                         }
                     }
                 }
@@ -171,13 +115,6 @@ public class CheckBundleExportsImports implements AnalyserTask {
 
         boolean errorReported = false;
         for(final Map.Entry<BundleDescriptor, Report> entry : reports.entrySet()) {
-            if ( !entry.getValue().importWithoutVersion.isEmpty() ) {
-                ctx.reportArtifactWarning(entry.getKey().getArtifact().getId(), " is importing package(s) " + getPackageInfo(entry.getValue().importWithoutVersion, false) + " without specifying a version range.");
-            }
-            if ( !entry.getValue().exportWithoutVersion.isEmpty() ) {
-                ctx.reportArtifactWarning(entry.getKey().getArtifact().getId(), " is exporting package(s) " + getPackageInfo(entry.getValue().importWithoutVersion, false) + " without a version.");
-            }
-
             if ( !entry.getValue().missingExports.isEmpty() ) {
                 ctx.reportArtifactError(entry.getKey().getArtifact().getId(), " is importing package(s) " + getPackageInfo(entry.getValue().missingExports, false) + " in start level " +
                         String.valueOf(entry.getKey().getArtifact().getStartOrder())
diff --git a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleUnversionedPackages.java b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleUnversionedPackages.java
new file mode 100644
index 0000000..c325350
--- /dev/null
+++ b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleUnversionedPackages.java
@@ -0,0 +1,106 @@
+/*
+ * 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.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.scanner.BundleDescriptor;
+import org.apache.sling.feature.scanner.PackageInfo;
+import org.osgi.framework.Version;
+
+
+public class CheckBundleUnversionedPackages 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 "bundle-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/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 fbec4ad..283edc8 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
@@ -1,5 +1,6 @@
 org.apache.sling.feature.analyser.task.impl.CheckApisJarsProperties
 org.apache.sling.feature.analyser.task.impl.CheckBundleExportsImports
+org.apache.sling.feature.analyser.task.impl.CheckBundleUnversionedPackages
 org.apache.sling.feature.analyser.task.impl.CheckBundleNativeCode
 org.apache.sling.feature.analyser.task.impl.CheckBundlesForConnect
 org.apache.sling.feature.analyser.task.impl.CheckBundlesForInitialContent
diff --git a/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckBundleUnversionedPackagesTest.java b/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckBundleUnversionedPackagesTest.java
new file mode 100644
index 0000000..3b80692
--- /dev/null
+++ b/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckBundleUnversionedPackagesTest.java
@@ -0,0 +1,87 @@
+/*
+ * 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 org.apache.sling.feature.Artifact;
+import org.apache.sling.feature.ArtifactId;
+import org.apache.sling.feature.Feature;
+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.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 java.net.URISyntaxException;
+
+import static org.junit.Assert.assertEquals;
+
+public class CheckBundleUnversionedPackagesTest {
+    private static File resourceRoot;
+
+    @BeforeClass
+    public static void setupClass() {
+        resourceRoot =
+                new File(CheckBundleExportsImportsTest.class.
+                        getResource("/test-content.zip").getFile()).getParentFile();
+    }
+
+    @Test
+    public void testId() {
+        assertEquals("bundle-unversioned-packages", new CheckBundleUnversionedPackages().getId());
+    }
+
+    @Test
+    public void testBundleUnversionedPackage() throws Exception {
+        CheckBundleUnversionedPackages t = new CheckBundleUnversionedPackages();
+
+        Feature f = new Feature(ArtifactId.fromMvnId("f:f:1"));
+        f.setComplete(true);
+        FeatureDescriptor fd = new FeatureDescriptorImpl(f);
+
+        fdAddBundle(fd, "g:b1:1", "test-bundle-unversioned.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 URISyntaxException, 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/resources/test-bundle-unversioned.jar b/src/test/resources/test-bundle-unversioned.jar
new file mode 100644
index 0000000..cb5d2de
Binary files /dev/null and b/src/test/resources/test-bundle-unversioned.jar differ