You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by kw...@apache.org on 2021/04/28 19:36:19 UTC

[jackrabbit-filevault] branch bugfix/JCRVLT-517-init-package-cache-in-osgi created (now 717e0ea)

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

kwin pushed a change to branch bugfix/JCRVLT-517-init-package-cache-in-osgi
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git.


      at 717e0ea  JCRVLT-517 fix package cache init in OSGi containers

This branch includes the following new commits:

     new 717e0ea  JCRVLT-517 fix package cache init in OSGi containers

The 1 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.


[jackrabbit-filevault] 01/01: JCRVLT-517 fix package cache init in OSGi containers

Posted by kw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

kwin pushed a commit to branch bugfix/JCRVLT-517-init-package-cache-in-osgi
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git

commit 717e0ea14e871ff077efe0cdf3ebfef3da4dfb96
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Wed Apr 28 21:34:37 2021 +0200

    JCRVLT-517 fix package cache init in OSGi containers
    
    get rid of lazy-loading of package cache in packages()
    improve test coverage
    fix another issue in FSInstallState not correctly persisting excludes
---
 .../packaging/registry/impl/FSInstallState.java    | 91 +++++++++++++++++++-
 .../packaging/registry/impl/FSPackageRegistry.java | 24 ++----
 .../registry/impl/FSInstallStateTest.java          | 62 ++++++++++++++
 .../registry/impl/FSPackageRegistryTest.java       | 98 ++++++++++++++++++++++
 .../vault/packaging/registry/impl/test-package.xml | 12 +++
 5 files changed, 267 insertions(+), 20 deletions(-)

diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSInstallState.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSInstallState.java
index 2435beb..0559f53 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSInstallState.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSInstallState.java
@@ -198,7 +198,10 @@ public class FSInstallState {
                 return null;
             }
             String packageId = doc.getAttribute(ATTR_PACKAGE_ID);
-            Path filePath = Paths.get(doc.getAttribute(ATTR_FILE_PATH));
+            Path filePath = null;
+            if (doc.hasAttribute(ATTR_FILE_PATH)) {
+                filePath = Paths.get(doc.getAttribute(ATTR_FILE_PATH));
+            }
             Long installTime = null;
             if (doc.hasAttribute(ATTR_INSTALLATION_TIME)) {
                 installTime = Long.valueOf(doc.getAttribute(ATTR_INSTALLATION_TIME));
@@ -287,7 +290,7 @@ public class FSInstallState {
                             DefaultPathFilter pf = new DefaultPathFilter(((Element) rule).getAttribute(ATTR_INCLUDE));
                             pfs.addInclude(pf);
                         } else if (((Element) rule).hasAttribute(ATTR_EXCLUDE)) {
-                            DefaultPathFilter pf = new DefaultPathFilter(((Element) rule).getAttribute(ATTR_INCLUDE));
+                            DefaultPathFilter pf = new DefaultPathFilter(((Element) rule).getAttribute(ATTR_EXCLUDE));
                             pfs.addExclude(pf);
                         }
                     }
@@ -326,7 +329,9 @@ public class FSInstallState {
             if (installTime != null) {
                 writer.writeAttribute(ATTR_INSTALLATION_TIME, Long.toString(installTime));
             }
-            writer.writeAttribute(ATTR_FILE_PATH, filePath.toString());
+            if (filePath != null) {
+                writer.writeAttribute(ATTR_FILE_PATH, filePath.toString());
+            }
             writer.writeAttribute(ATTR_EXTERNAL, Boolean.toString(external));
             writer.writeAttribute(ATTR_PACKAGE_STATUS, status.name().toLowerCase());
 
@@ -417,4 +422,84 @@ public class FSInstallState {
     public Properties getProperties() {
         return properties;
     }
+
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + ((dependencies == null) ? 0 : dependencies.hashCode());
+        result = prime * result + (external ? 1231 : 1237);
+        result = prime * result + ((filePath == null) ? 0 : filePath.hashCode());
+        result = prime * result + ((filter == null) ? 0 : filter.hashCode());
+        result = prime * result + ((installTime == null) ? 0 : installTime.hashCode());
+        result = prime * result + ((packageId == null) ? 0 : packageId.hashCode());
+        result = prime * result + ((properties == null) ? 0 : properties.hashCode());
+        result = prime * result + (int) (size ^ (size >>> 32));
+        result = prime * result + ((status == null) ? 0 : status.hashCode());
+        result = prime * result + ((subPackages == null) ? 0 : subPackages.hashCode());
+        return result;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj)
+            return true;
+        if (obj == null)
+            return false;
+        if (getClass() != obj.getClass())
+            return false;
+        FSInstallState other = (FSInstallState) obj;
+        if (dependencies == null) {
+            if (other.dependencies != null)
+                return false;
+        } else if (!dependencies.equals(other.dependencies))
+            return false;
+        if (external != other.external)
+            return false;
+        if (filePath == null) {
+            if (other.filePath != null)
+                return false;
+        } else if (!filePath.equals(other.filePath))
+            return false;
+        if (filter == null) {
+            if (other.filter != null)
+                return false;
+        } else if (!filter.equals(other.filter))
+            return false;
+        if (installTime == null) {
+            if (other.installTime != null)
+                return false;
+        } else if (!installTime.equals(other.installTime))
+            return false;
+        if (packageId == null) {
+            if (other.packageId != null)
+                return false;
+        } else if (!packageId.equals(other.packageId))
+            return false;
+        if (properties == null) {
+            if (other.properties != null)
+                return false;
+        } else if (!properties.equals(other.properties))
+            return false;
+        if (size != other.size)
+            return false;
+        if (status != other.status)
+            return false;
+        if (subPackages == null) {
+            if (other.subPackages != null)
+                return false;
+        } else if (!subPackages.equals(other.subPackages))
+            return false;
+        return true;
+    }
+
+    @Override
+    public String toString() {
+        return "FSInstallState [" + (packageId != null ? "packageId=" + packageId + ", " : "")
+                + (status != null ? "status=" + status + ", " : "") + (filePath != null ? "filePath=" + filePath + ", " : "") + "external="
+                + external + ", " + (dependencies != null ? "dependencies=" + dependencies + ", " : "")
+                + (subPackages != null ? "subPackages=" + subPackages + ", " : "")
+                + (installTime != null ? "installTime=" + installTime + ", " : "") + "size=" + size + ", "
+                + (filter != null ? "filter=" + filter + ", " : "") + (properties != null ? "properties=" + properties : "") + "]";
+    }
 }
\ No newline at end of file
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSPackageRegistry.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSPackageRegistry.java
index f8c19c6..6152def 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSPackageRegistry.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSPackageRegistry.java
@@ -87,7 +87,7 @@ import org.slf4j.LoggerFactory;
 @Designate(ocd = FSPackageRegistry.Config.class)
 public class FSPackageRegistry extends AbstractPackageRegistry {
 
-    private static final String REPOSITORY_HOME = "repository.home";
+    protected static final String REPOSITORY_HOME = "repository.home";
 
     /**
      * default logger
@@ -97,7 +97,7 @@ public class FSPackageRegistry extends AbstractPackageRegistry {
     /**
      * Suffixes for metadata files
      */
-    private final String[] META_SUFFIXES = {"xml"};
+    private static final String[] META_SUFFIXES = {"xml"};
 
     private Map<PackageId, FSInstallState> stateCache = new ConcurrentHashMap<>();
 
@@ -106,9 +106,6 @@ public class FSPackageRegistry extends AbstractPackageRegistry {
      */
     private Map<Path, PackageId> pathIdMapping = new ConcurrentHashMap<>();
 
-
-    private boolean packagesInitializied = false;
-
     @Reference
     private PackageEventDispatcher dispatcher;
 
@@ -175,7 +172,7 @@ public class FSPackageRegistry extends AbstractPackageRegistry {
     }
 
     @Activate
-    public void activate(BundleContext context, Config config) {
+    public void activate(BundleContext context, Config config) throws IOException {
         this.homeDir = context.getProperty(REPOSITORY_HOME) != null ? ( 
                 new File(config.homePath()).isAbsolute() ? new File(config.homePath()) : new File(context.getProperty(REPOSITORY_HOME) + "/" + config.homePath())) : 
                 context.getDataFile(config.homePath());
@@ -185,6 +182,7 @@ public class FSPackageRegistry extends AbstractPackageRegistry {
         log.info("Jackrabbit Filevault FS Package Registry initialized with home location {}", this.homeDir.getPath());
         this.scope = InstallationScope.valueOf(config.scope());
         this.securityConfig = new AbstractPackageRegistry.SecurityConfig(config.authIdsForHookExecution(), config.authIdsForRootInstallation());
+        loadPackageCache();
     }
 
     @ObjectClassDefinition(
@@ -381,10 +379,7 @@ public class FSPackageRegistry extends AbstractPackageRegistry {
     public PackageId register(@NotNull InputStream in, boolean replace) throws IOException, PackageExistsException {
       return register(in, replace, null);
     }
-    
-    /**
-     * {@inheritDoc}
-     */
+
     @NotNull
     private PackageId register(@NotNull InputStream in, boolean replace, Dependency autoDependency) throws IOException, PackageExistsException {
         ZipVaultPackage pkg = upload(in, replace);
@@ -480,10 +475,7 @@ public class FSPackageRegistry extends AbstractPackageRegistry {
         }
     }
 
-    /**
-     * {@inheritDoc}
-     */
-    public ZipVaultPackage upload(InputStream in, boolean replace)
+    protected ZipVaultPackage upload(InputStream in, boolean replace)
             throws IOException, PackageExistsException {
 
         MemoryArchive archive = new MemoryArchive(false);
@@ -637,7 +629,7 @@ public class FSPackageRegistry extends AbstractPackageRegistry {
     @NotNull
     @Override
     public Set<PackageId> packages() throws IOException {
-        return packagesInitializied ? stateCache.keySet() : loadPackageCache();
+        return stateCache.keySet();
     }
 
     /**
@@ -660,13 +652,11 @@ public class FSPackageRegistry extends AbstractPackageRegistry {
                 if (id != null) {
                     cacheEntries.put(id, state);
                     idMapping.put(state.getFilePath(), id);
-                    
                 }
             }
         }
         stateCache.putAll(cacheEntries);
         pathIdMapping.putAll(idMapping);
-        packagesInitializied = true;
         return cacheEntries.keySet();
     }
 
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSInstallStateTest.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSInstallStateTest.java
new file mode 100644
index 0000000..491bde0
--- /dev/null
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSInstallStateTest.java
@@ -0,0 +1,62 @@
+/*
+ * 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.jackrabbit.vault.packaging.registry.impl;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Collections;
+
+import org.apache.jackrabbit.vault.fs.api.PathFilterSet;
+import org.apache.jackrabbit.vault.fs.config.ConfigurationException;
+import org.apache.jackrabbit.vault.fs.config.DefaultWorkspaceFilter;
+import org.apache.jackrabbit.vault.fs.filter.DefaultPathFilter;
+import org.apache.jackrabbit.vault.packaging.Dependency;
+import org.apache.jackrabbit.vault.packaging.PackageId;
+import org.apache.jackrabbit.vault.packaging.SubPackageHandling;
+import org.junit.Test;
+
+public class FSInstallStateTest {
+
+    @Test
+    public void testSaveLoad() throws IOException, ConfigurationException {
+        PackageId packageId = new PackageId("group", "name", "1.1.1");
+        FSInstallState fsInstallState = new FSInstallState(packageId, FSPackageStatus.REGISTERED);
+        DefaultWorkspaceFilter filter = new DefaultWorkspaceFilter();
+        PathFilterSet pathFilterSet = new PathFilterSet("/apps/mytest");
+        pathFilterSet.addExclude(new DefaultPathFilter("/apps/mytest/exclude"));
+        pathFilterSet.addInclude(new DefaultPathFilter("/apps/mytest/include"));
+        pathFilterSet.seal();
+        filter.add(pathFilterSet);
+        fsInstallState.withFilter(filter);
+        fsInstallState.withDependencies(Collections.singleton(new Dependency(new PackageId("group", "other", "1.0.0"))));
+        fsInstallState.withInstallTime(1234L);
+        fsInstallState.withSize(333);
+        fsInstallState.withSubPackages(Collections.singletonMap(new PackageId("group", "subpackage", "1.0.0"), SubPackageHandling.Option.FORCE_EXTRACT));
+        try (ByteArrayOutputStream output = new ByteArrayOutputStream()) {
+            fsInstallState.save(output);
+            try (InputStream input = new ByteArrayInputStream(output.toByteArray())) {
+                FSInstallState fsInstallState2 = FSInstallState.fromStream(input, "systemId");
+                assertEquals(fsInstallState, fsInstallState2);
+            }
+        }
+    }
+
+}
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSPackageRegistryTest.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSPackageRegistryTest.java
new file mode 100644
index 0000000..cd92f01
--- /dev/null
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/registry/impl/FSPackageRegistryTest.java
@@ -0,0 +1,98 @@
+/*
+ * 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.jackrabbit.vault.packaging.registry.impl;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.jackrabbit.vault.packaging.NoSuchPackageException;
+import org.apache.jackrabbit.vault.packaging.PackageExistsException;
+import org.apache.jackrabbit.vault.packaging.PackageId;
+import org.apache.jackrabbit.vault.packaging.registry.impl.FSPackageRegistry.Config;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.mockito.Mockito;
+import org.osgi.framework.BundleContext;
+import org.osgi.util.converter.Converter;
+import org.osgi.util.converter.Converters;
+
+
+public class FSPackageRegistryTest {
+
+    @Rule
+    public TemporaryFolder tmpFolder = new TemporaryFolder();
+    
+    private static final PackageId TEST_PACKAGE_ID = new PackageId("test", "test-package-with-etc", "1.0");
+    
+    private void copyResourceStreamToFile(Path targetFile, String name) throws IOException {
+        try (InputStream in = getClass().getResourceAsStream(name)) {
+            Files.copy(in, targetFile, StandardCopyOption.REPLACE_EXISTING);
+        }
+    }
+
+    private Path getTempRegistryHomeWithPackage(String packageName, String packageMetadataName) throws IOException {
+        Path tmpDir = tmpFolder.newFolder().toPath();
+        copyResourceStreamToFile(tmpDir.resolve("package1.zip"), packageName);
+        copyResourceStreamToFile(tmpDir.resolve("package1.zip.xml"), packageMetadataName);
+        return tmpDir;
+    }
+
+    @Test
+    public void testRegisterAndRemove() throws IOException, PackageExistsException, NoSuchPackageException {
+        FSPackageRegistry registry = createRegistryWithDefaultConstructor(tmpFolder.newFolder().toPath());
+        try (InputStream in = getClass().getResourceAsStream("test-package.zip")) {
+            registry.register(in, false);
+        }
+        assertTrue(registry.contains(TEST_PACKAGE_ID));
+        registry.remove(TEST_PACKAGE_ID);
+        assertFalse(registry.contains(TEST_PACKAGE_ID));
+    }
+
+    @Test
+    public void testCacheInitializedAfterOSGiActivate() throws IOException {
+         new FSPackageRegistry();
+        Path registryHomeDir = getTempRegistryHomeWithPackage("test-package.zip", "test-package.xml");
+        FSPackageRegistry registry = createRegistryWithDefaultConstructor(registryHomeDir);
+        assertTrue(registry.contains(TEST_PACKAGE_ID));
+        assertEquals(Collections.singleton(TEST_PACKAGE_ID), registry.packages());
+    }
+
+    private FSPackageRegistry createRegistryWithDefaultConstructor(Path homePath) throws IOException {
+        FSPackageRegistry registry = new FSPackageRegistry();
+        BundleContext context = Mockito.mock(BundleContext.class);
+        Mockito.when(context.getProperty(FSPackageRegistry.REPOSITORY_HOME)).thenReturn(tmpFolder.getRoot().toString());
+        Converter converter = Converters.standardConverter();
+        Map<String, Object> map = new HashMap<>();
+        map.put("homePath", homePath.toString());
+        map.put("authIdsForHookExecution", new String[0]);
+        map.put("authIdsForRootInstallation", new String[0]);
+        Config config = converter.convert(map).to(Config.class);
+        registry.activate(context, config);
+        return registry;
+    }
+}
diff --git a/vault-core/src/test/resources/org/apache/jackrabbit/vault/packaging/registry/impl/test-package.xml b/vault-core/src/test/resources/org/apache/jackrabbit/vault/packaging/registry/impl/test-package.xml
new file mode 100644
index 0000000..d1fba7d
--- /dev/null
+++ b/vault-core/src/test/resources/org/apache/jackrabbit/vault/packaging/registry/impl/test-package.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<registryMetadata packageid="test:test-package-with-etc:1.0" size="-1" filepath="/var/folders/rm/vlg2h6m16mb0f65djmnb12xr0000gq/T/junit1443483155243547543/junit9008025097740758307/home/test/test-package-with-etc-1.0.zip" external="false" packagestatus="registered">
+    <workspacefilter>
+        <filter root="/etc">
+            <rule include="/etc"/>
+            <rule include="/etc/clientlibs"/>
+            <rule include="/etc/clientlibs/granite"/>
+            <rule include="/etc/clientlibs/granite/test(/.*)?"/>
+        </filter>
+    </workspacefilter>
+    <packageproperties createdBy="admin" name="test-package-with-etc" lastModified="2017-05-16T17:23:18.391+09:00" lastModifiedBy="admin" acHandling="MERGE_PRESERVE" created="2017-05-16T17:23:18.655+09:00" buildCount="1" version="1.0" dependencies="" packageFormatVersion="2" group="test" lastWrappedBy="admin"/>
+</registryMetadata>