You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by ro...@apache.org on 2021/05/09 13:59:17 UTC

[felix-dev] branch master updated (a195e1e -> 98d3533)

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

rotty3000 pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/felix-dev.git.


    from a195e1e  [FELIX-6409] FileInstall logs useless messages due to incorrect interpolations
     new bbaa489  [FELIX-6410] Set the READ_ONLY attribute for configurations
     new 98d3533  [FELIX-6412] Use Configuration.updateIfDifferent

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:
 fileinstall/pom.xml                                |  14 +-
 .../fileinstall/internal/ConfigInstaller.java      | 223 +++++++++++++++++++--
 .../fileinstall/internal/ConfigInstallerTest.java  | 164 ++++++++++++---
 3 files changed, 352 insertions(+), 49 deletions(-)

[felix-dev] 01/02: [FELIX-6410] Set the READ_ONLY attribute for configurations

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

rotty3000 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/felix-dev.git

commit bbaa489dbbb9c3358b961d87d2200cfdbc4db15c
Author: Raymond Augé <ro...@apache.org>
AuthorDate: Sun May 9 09:53:18 2021 -0400

    [FELIX-6410] Set the READ_ONLY attribute for configurations
    
    Signed-off-by: Raymond Augé <ro...@apache.org>
---
 fileinstall/pom.xml                                |  14 +-
 .../fileinstall/internal/ConfigInstaller.java      | 192 +++++++++++++++++++--
 .../fileinstall/internal/ConfigInstallerTest.java  | 152 +++++++++++++---
 3 files changed, 316 insertions(+), 42 deletions(-)

diff --git a/fileinstall/pom.xml b/fileinstall/pom.xml
index 2249611..a9b9598 100644
--- a/fileinstall/pom.xml
+++ b/fileinstall/pom.xml
@@ -23,7 +23,7 @@
     <version>6</version>
     <relativePath>../pom/pom.xml</relativePath>
   </parent>
-	
+
   <modelVersion>4.0.0</modelVersion>
   <packaging>jar</packaging>
   <name>Apache Felix File Install</name>
@@ -49,8 +49,14 @@
     </dependency>
     <dependency>
       <groupId>org.osgi</groupId>
-      <artifactId>org.osgi.compendium</artifactId>
-      <version>4.3.1</version>
+      <artifactId>org.osgi.service.cm</artifactId>
+      <version>1.6.0</version>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.osgi</groupId>
+      <artifactId>org.osgi.service.log</artifactId>
+      <version>1.3.0</version>
       <scope>provided</scope>
     </dependency>
     <dependency>
@@ -104,7 +110,7 @@
             </Private-Package>
             <Import-Package>
                 org.osgi.service.log;resolution:=optional,
-                org.osgi.service.cm;resolution:=optional,
+                org.osgi.service.cm;version="[1.5,2)";resolution:=optional,
                 !org.apache.felix.fileinstall,
                 !org.apache.felix.cm.file,
                 !org.apache.felix.utils.*,
diff --git a/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java b/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java
index 82ed61f..9b76f8a 100644
--- a/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java
+++ b/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java
@@ -18,11 +18,33 @@
  */
 package org.apache.felix.fileinstall.internal;
 
-import java.io.*;
+import java.io.BufferedInputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.io.Reader;
+import java.io.Writer;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.attribute.PosixFilePermission;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Dictionary;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.Hashtable;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
 
 import org.apache.felix.fileinstall.ArtifactInstaller;
 import org.apache.felix.fileinstall.ArtifactListener;
@@ -30,8 +52,13 @@ import org.apache.felix.fileinstall.internal.Util.Logger;
 import org.apache.felix.utils.collections.DictionaryAsMap;
 import org.apache.felix.utils.properties.InterpolationHelper;
 import org.apache.felix.utils.properties.TypedProperties;
-import org.osgi.framework.*;
-import org.osgi.service.cm.*;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.Constants;
+import org.osgi.framework.ServiceRegistration;
+import org.osgi.service.cm.Configuration;
+import org.osgi.service.cm.ConfigurationAdmin;
+import org.osgi.service.cm.ConfigurationEvent;
+import org.osgi.service.cm.ConfigurationListener;
 
 /**
  * ArtifactInstaller for configurations.
@@ -43,13 +70,65 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
     private final ConfigurationAdmin configAdmin;
     private final FileInstall fileInstall;
     private final Map<String, String> pidToFile = new HashMap<>();
-    private ServiceRegistration registration;
+    private final Method addAttributesMethod;
+    private final Method getAttributesMethod;
+    private final Method removeAttributesMethod;
+    private final Object READ_ONLY_ATTRIBUTE_ARRAY;
+    private final boolean osWin;
+    private ServiceRegistration<?> registration;
 
     ConfigInstaller(BundleContext context, ConfigurationAdmin configAdmin, FileInstall fileInstall)
     {
         this.context = context;
         this.configAdmin = configAdmin;
         this.fileInstall = fileInstall;
+
+        Method aaMethod = null;
+        Method gaMethod = null;
+        Method raMethod = null;
+
+        if (this.configAdmin != null) {
+            for (Method method : Configuration.class.getDeclaredMethods())
+            {
+                if ("addAttributes".equals(method.getName()))
+                {
+                    aaMethod = method;
+                }
+                else if ("getAttributes".equals(method.getName()))
+                {
+                    gaMethod = method;
+                }
+                else if ("removeAttributes".equals(method.getName()))
+                {
+                    raMethod = method;
+                }
+            }
+        }
+
+        this.addAttributesMethod = aaMethod;
+        this.getAttributesMethod = gaMethod;
+        this.removeAttributesMethod = raMethod;
+
+        Object roAttributesArray = null;
+        if (this.addAttributesMethod != null)
+        {
+            try {
+                Class<? extends Enum> attr = (Class<? extends Enum>)context.getBundle()
+                    .loadClass("org.osgi.service.cm.Configuration$ConfigurationAttribute");
+
+                Object attrArray = Array.newInstance(attr, 1);
+                Array.set(attrArray, 0, Enum.valueOf(attr, "READ_ONLY"));
+
+                roAttributesArray = attrArray;
+            }
+            catch (ClassNotFoundException cnfe) {
+                throw new IllegalStateException(
+                    "Cannot find the enum org.osgi.service.cm.Configuration.ConfigurationAttribute", cnfe);
+            }
+        }
+
+        this.READ_ONLY_ATTRIBUTE_ARRAY = roAttributesArray;
+        this.osWin = System.getProperty("os.name").toLowerCase().contains("win");
     }
 
     public void init()
@@ -63,12 +142,12 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
                 this, null);
         try
         {
-            Configuration[] configs = configAdmin.listConfigurations(null);
+            Configuration[] configs = getConfigurationAdmin().listConfigurations(null);
             if (configs != null)
             {
                 for (Configuration config : configs)
                 {
-                    Dictionary dict = config.getProperties();
+                    Dictionary<?,?> dict = config.getProperties();
                     String fileName = dict != null ? (String) dict.get( DirectoryWatcher.FILENAME ) : null;
                     if (fileName != null)
                     {
@@ -147,11 +226,17 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
                 Configuration config = getConfigurationAdmin().getConfiguration(
                                             configurationEvent.getPid(),
                                             "?");
-                Dictionary dict = config.getProperties();
+                Dictionary<?,?> dict = config.getProperties();
                 String fileName = dict != null ? (String) dict.get( DirectoryWatcher.FILENAME ) : null;
                 File file = fileName != null ? fromConfigKey(fileName) : null;
                 if( file != null && file.isFile() && canHandle( file ) ) {
                     pidToFile.put(config.getPid(), fileName);
+                    if (!file.canWrite())
+                    {
+                        Util.log(context, Logger.LOG_WARNING, "File is not writeable: " + fileName, null);
+                        return;
+                    }
+
                     TypedProperties props = new TypedProperties( bundleSubstitution() );
                     try (Reader r = new InputStreamReader(new FileInputStream(file), encoding()))
                     {
@@ -168,7 +253,7 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
                             propertiesToRemove.add(key);
                         }
                     }
-                    for( Enumeration e  = dict.keys(); e.hasMoreElements(); )
+                    for( Enumeration<?> e  = dict.keys(); e.hasMoreElements(); )
                     {
                         String key = e.nextElement().toString();
                         if( !Constants.SERVICE_PID.equals(key)
@@ -194,7 +279,7 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
             }
             catch (Exception e)
             {
-                Util.log( context, Logger.LOG_INFO, "Unable to save configuration", e );
+                Util.log( context, Logger.LOG_ERROR, "Unable to save configuration", e );
             }
         }
 
@@ -286,12 +371,14 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
         String pid[] = parsePid(f.getName());
         Configuration config = getConfiguration(toConfigKey(f), pid[0], pid[1]);
 
+        setReadOnly(pid, config, f);
+
         Dictionary<String, Object> props = config.getProperties();
         Hashtable<String, Object> old = props != null ? new Hashtable<String, Object>(new DictionaryAsMap<>(props)) : null;
         if (old != null) {
-        	old.remove( DirectoryWatcher.FILENAME );
-        	old.remove( Constants.SERVICE_PID );
-        	old.remove( ConfigurationAdmin.SERVICE_FACTORYPID );
+            old.remove( DirectoryWatcher.FILENAME );
+            old.remove( Constants.SERVICE_PID );
+            old.remove( ConfigurationAdmin.SERVICE_FACTORYPID );
         }
 
         if( !ht.equals( old ) )
@@ -304,7 +391,7 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
             } else {
                 Util.log(context, Logger.LOG_INFO, "Updating configuration {" + pid[0]
                         + (pid[1] == null ? "" : "-" + pid[1])
-						+ "} from " + f.getName(), null);
+                        + "} from " + f.getName(), null);
             }
             config.update(ht);
             return true;
@@ -413,6 +500,83 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
         };
     }
 
+    boolean isReadOnly(Configuration configuration) throws IOException {
+        if (getAttributesMethod == null) {
+            return false;
+        }
+
+        try {
+            Set<? extends Enum<?>> attributes = (Set<? extends Enum<?>>)getAttributesMethod.invoke(configuration);
+
+            if (attributes.isEmpty()) {
+                return false;
+            }
+
+            return attributes.iterator().next().name().equals("READ_ONLY");
+        }
+        catch (InvocationTargetException ite) {
+            Throwable targetException = ite.getTargetException();
+            if (targetException instanceof IOException) {
+                throw (IOException)targetException;
+            }
+
+            throw new RuntimeException(targetException);
+        }
+        catch (Throwable t) {
+            throw new RuntimeException(t);
+        }
+    }
+
+    boolean setReadOnly(String[] pid, Configuration configuration, File f) throws IOException {
+        if (addAttributesMethod == null) {
+            return false;
+        }
+
+        try {
+            if (!canWrite(f)) {
+                Util.log(context, Logger.LOG_INFO, "Adding  READ_ONLY attribute to configuration {" + pid[0]
+                        + (pid[1] == null ? "" : "-" + pid[1])
+                        + "} from " + f.getName(), null);
+                addAttributesMethod.invoke(configuration, READ_ONLY_ATTRIBUTE_ARRAY);
+
+                return true;
+            }
+            else if (isReadOnly(configuration)) {
+                Util.log(context, Logger.LOG_INFO, "Removing  READ_ONLY attribute from configuration {" + pid[0]
+                        + (pid[1] == null ? "" : "-" + pid[1])
+                        + "} from " + f.getName(), null);
+                removeAttributesMethod.invoke(configuration, READ_ONLY_ATTRIBUTE_ARRAY);
+
+                return true;
+            }
+        }
+        catch (InvocationTargetException ite) {
+            Throwable targetException = ite.getTargetException();
+            if (targetException instanceof IOException) {
+                throw (IOException)targetException;
+            }
+
+            throw new RuntimeException(targetException);
+        }
+        catch (Throwable t) {
+            throw new RuntimeException(t);
+        }
+
+        return false;
+    }
+
+    private boolean canWrite(File f) throws IOException {
+        if (osWin) {
+            return f.canWrite();
+        }
+
+        Set<PosixFilePermission> posixFilePermissions = Files.getPosixFilePermissions(f.toPath());
+
+        return posixFilePermissions.contains(PosixFilePermission.OWNER_WRITE) ||
+            posixFilePermissions.contains(PosixFilePermission.GROUP_WRITE) ||
+            posixFilePermissions.contains(PosixFilePermission.OTHERS_WRITE);
+    }
+
     private String escapeFilterValue(String s) {
         return s.replaceAll("[(]", "\\\\(").
                 replaceAll("[)]", "\\\\)").
diff --git a/fileinstall/src/test/java/org/apache/felix/fileinstall/internal/ConfigInstallerTest.java b/fileinstall/src/test/java/org/apache/felix/fileinstall/internal/ConfigInstallerTest.java
index 5878026..6b6a12c 100644
--- a/fileinstall/src/test/java/org/apache/felix/fileinstall/internal/ConfigInstallerTest.java
+++ b/fileinstall/src/test/java/org/apache/felix/fileinstall/internal/ConfigInstallerTest.java
@@ -23,6 +23,7 @@ import java.io.FileOutputStream;
 import java.io.OutputStream;
 import java.nio.file.Files;
 import java.nio.file.Paths;
+import java.util.Collections;
 import java.util.Dictionary;
 import java.util.Hashtable;
 import java.util.concurrent.atomic.AtomicReference;
@@ -36,6 +37,7 @@ import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
 import org.osgi.service.cm.Configuration;
+import org.osgi.service.cm.Configuration.ConfigurationAttribute;
 import org.osgi.service.cm.ConfigurationAdmin;
 import org.osgi.service.cm.ConfigurationEvent;
 
@@ -87,20 +89,21 @@ public class ConfigInstallerTest extends TestCase {
         String pid = file.getName().substring(0, file.getName().indexOf(".config"));
 
         Capture<Dictionary<String, Object>> props = new Capture<>();
+        EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes();
+        EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes();
         EasyMock.expect(mockConfigurationAdmin.listConfigurations((String) EasyMock.anyObject()))
                 .andReturn(null);
         EasyMock.expect(mockConfigurationAdmin.getConfiguration(pid, "?"))
                 .andReturn(mockConfiguration);
+        EasyMock.expect(mockConfiguration.getAttributes()).andReturn(Collections.emptySet());
         EasyMock.expect(mockConfiguration.getProperties())
                 .andReturn(null);
         EasyMock.expect(mockBundleContext.getProperty((String) EasyMock.anyObject()))
                 .andReturn(null)
                 .anyTimes();
-        EasyMock.expect(mockBundleContext.getBundle())
-                .andReturn(mockBundle);
         mockConfiguration.update(EasyMock.capture(props));
         EasyMock.expectLastCall();
-        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext);
+        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle);
 
         ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() );
         ci.install(file);
@@ -120,20 +123,22 @@ public class ConfigInstallerTest extends TestCase {
         String pid = file.getName().substring(0, file.getName().indexOf(".config"));
 
         Capture<Dictionary<String, Object>> props = new Capture<>();
+        EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes();
+        EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes();
         EasyMock.expect(mockConfigurationAdmin.listConfigurations((String) EasyMock.anyObject()))
                 .andReturn(null);
         EasyMock.expect(mockConfigurationAdmin.getConfiguration(pid, "?"))
                 .andReturn(mockConfiguration);
+        EasyMock.expect(mockConfiguration.getAttributes())
+            .andReturn(Collections.emptySet());
         EasyMock.expect(mockConfiguration.getProperties())
                 .andReturn(null);
         EasyMock.expect(mockBundleContext.getProperty((String) EasyMock.anyObject()))
                 .andReturn(null)
                 .anyTimes();
-        EasyMock.expect(mockBundleContext.getBundle())
-                .andReturn(mockBundle);
         mockConfiguration.update(EasyMock.capture(props));
         EasyMock.expectLastCall();
-        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext);
+        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle);
 
         ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() );
         ci.install(file);
@@ -146,11 +151,13 @@ public class ConfigInstallerTest extends TestCase {
 
     public void testGetNewFactoryConfiguration() throws Exception
     {
+        EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes();
+        EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes();
         EasyMock.expect(mockConfigurationAdmin.listConfigurations((String) EasyMock.anyObject()))
                     .andReturn(null);
         EasyMock.expect(mockConfigurationAdmin.createFactoryConfiguration( "pid", "?" ))
                     .andReturn(mockConfiguration);
-        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext);
+        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle);
 
         ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() );
 
@@ -162,11 +169,13 @@ public class ConfigInstallerTest extends TestCase {
 
     public void testGetExistentFactoryConfiguration() throws Exception
     {
+        EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes();
+        EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes();
         EasyMock.expect(mockConfigurationAdmin.listConfigurations((String) EasyMock.anyObject()))
                         .andReturn(null);
         EasyMock.expect(mockConfigurationAdmin.createFactoryConfiguration( "pid", "?" ))
                         .andReturn(mockConfiguration);
-        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext);
+        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle);
 
         ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() );
 
@@ -178,11 +187,13 @@ public class ConfigInstallerTest extends TestCase {
 
     public void testGetExistentNoFactoryConfiguration() throws Exception
     {
+        EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes();
+        EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes();
         EasyMock.expect(mockConfigurationAdmin.listConfigurations((String) EasyMock.anyObject()))
                         .andReturn(null);
         EasyMock.expect(mockConfigurationAdmin.getConfiguration( "pid", "?" ))
                         .andReturn(mockConfiguration);
-        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext);
+        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle);
 
         ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() );
 
@@ -195,13 +206,16 @@ public class ConfigInstallerTest extends TestCase {
     public void testDeleteConfig() throws Exception
     {
         mockConfiguration.delete();
-        EasyMock.expect(mockBundleContext.getProperty(DirectoryWatcher.LOG_LEVEL)).andReturn(null);
         EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes();
+        EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes();
+        EasyMock.expect(mockBundleContext.getProperty((String) EasyMock.anyObject()))
+            .andReturn(null)
+            .anyTimes();
         EasyMock.expect(mockConfigurationAdmin.listConfigurations((String) EasyMock.anyObject()))
                         .andReturn(null);
         EasyMock.expect(mockConfigurationAdmin.getConfiguration("pid", "?" ))
                         .andReturn(mockConfiguration);
-        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext);
+        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle);
 
         ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() );
 
@@ -220,6 +234,9 @@ public class ConfigInstallerTest extends TestCase {
 
         final Capture<Dictionary<String, Object>> props = new Capture<>();
 
+        EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes();
+        EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes();
+        EasyMock.expect(mockConfiguration.getAttributes()).andReturn(Collections.emptySet());
         EasyMock.expect(mockConfiguration.getProperties())
                 .andReturn(null);
         EasyMock.expect(mockBundleContext.getProperty((String) EasyMock.anyObject()))
@@ -246,7 +263,7 @@ public class ConfigInstallerTest extends TestCase {
         EasyMock.expect(mockConfiguration.getPid())
                 .andReturn(pid);
 
-        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, sr);
+        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle, sr);
 
         ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() );
         ci.install(file);
@@ -264,6 +281,8 @@ public class ConfigInstallerTest extends TestCase {
 
         Capture<Dictionary<String, Object>> props = new Capture<>();
 
+        EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes();
+        EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes();
         EasyMock.expect(mockConfiguration.getProperties())
                 .andReturn(null);
         EasyMock.expect(mockBundleContext.getProperty((String) EasyMock.anyObject()))
@@ -278,7 +297,7 @@ public class ConfigInstallerTest extends TestCase {
         mockConfiguration.update(EasyMock.capture(props));
         EasyMock.expectLastCall();
 
-        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, sr);
+        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle, sr);
 
         ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() );
 
@@ -298,6 +317,8 @@ public class ConfigInstallerTest extends TestCase {
         Dictionary<String, Object> props = new Hashtable<>();
         props.put(DirectoryWatcher.FILENAME, file.toURI().toString());
 
+        EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes();
+        EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes();
         EasyMock.expect(mockConfiguration.getProperties())
                 .andReturn(props);
         EasyMock.expect(mockBundleContext.getProperty((String) EasyMock.anyObject()))
@@ -314,7 +335,7 @@ public class ConfigInstallerTest extends TestCase {
         mockConfiguration.update(EasyMock.capture(propsCapture));
         EasyMock.expectLastCall();
 
-        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, sr);
+        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle, sr);
 
         ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() );
 
@@ -335,11 +356,14 @@ public class ConfigInstallerTest extends TestCase {
 
         ServiceReference<ConfigurationAdmin> sr = EasyMock.createMock(ServiceReference.class);
 
+        EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes();
+        EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes();
         EasyMock.expect(mockBundleContext.getProperty((String) EasyMock.anyObject()))
                 .andReturn(null)
                 .anyTimes();
 
         final Configuration cachingPersistenceConfiguration = EasyMock.createMock(Configuration.class);
+        EasyMock.expect(cachingPersistenceConfiguration.getAttributes()).andReturn(Collections.emptySet()).once();
 
         final Dictionary<String, Object> cachedProps = new Hashtable<String, Object>() {
             {
@@ -364,6 +388,7 @@ public class ConfigInstallerTest extends TestCase {
                 .anyTimes();
 
         final Configuration newConfiguration = EasyMock.createMock(Configuration.class);
+        EasyMock.expect(newConfiguration.getAttributes()).andReturn(Collections.emptySet()).once();
 
         EasyMock.expect(mockConfigurationAdmin.listConfigurations((String) EasyMock.anyObject()))
                 .andReturn(new Configuration[] { newConfiguration });
@@ -379,17 +404,17 @@ public class ConfigInstallerTest extends TestCase {
 
         cachingPersistenceConfiguration.update(EasyMock.capture(props));
         EasyMock.expectLastCall()
-                .andAnswer(new IAnswer<Object>() {
+                .andAnswer(new IAnswer<Boolean>() {
                     @Override
-                    public Object answer() throws Throwable {
+                    public Boolean answer() throws Throwable {
 
                         cachedProps.put("networkInterface", props.getValue().get("networkInterface"));
 
-                        return null;
+                        return Boolean.TRUE;
                    }
                }).anyTimes();
 
-        EasyMock.replay(mockBundleContext, mockConfigurationAdmin, cachingPersistenceConfiguration, newConfiguration, sr);
+        EasyMock.replay(mockBundleContext, mockConfigurationAdmin, mockBundle, cachingPersistenceConfiguration, newConfiguration, sr);
 
         ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() );
 
@@ -406,11 +431,13 @@ public class ConfigInstallerTest extends TestCase {
 
     public void testSetConfiguration() throws Exception
     {
+        EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes();
+        EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes();
         EasyMock.expect(mockBundleContext.getProperty(DirectoryWatcher.CONFIG_ENCODING)).andReturn(null);
         EasyMock.expect(mockBundleContext.getProperty(DirectoryWatcher.LOG_DEFAULT)).andReturn(null);
         EasyMock.expect(mockBundleContext.getProperty(DirectoryWatcher.LOG_LEVEL)).andReturn(null);
-        EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes();
         EasyMock.expect(mockConfiguration.getProperties()).andReturn(new Hashtable<String, Object>());
+        EasyMock.expect(mockConfiguration.getAttributes()).andReturn(Collections.emptySet());
         EasyMock.reportMatcher(new IArgumentMatcher()
         {
             public boolean matches( Object argument )
@@ -428,7 +455,7 @@ public class ConfigInstallerTest extends TestCase {
                         .andReturn(null);
         EasyMock.expect(mockConfigurationAdmin.getConfiguration("firstcfg", "?"))
                         .andReturn(mockConfiguration);
-        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext);
+        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle);
 
         ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() );
 
@@ -436,12 +463,14 @@ public class ConfigInstallerTest extends TestCase {
 
         EasyMock.verify(mockConfiguration, mockConfigurationAdmin, mockBundleContext);
     }
-    
-    public void testShouldSaveConfig() 
+
+    public void testShouldSaveConfig() throws Exception
     {
         final AtomicReference<Boolean> disable = new AtomicReference<Boolean>();
         final AtomicReference<Boolean> enable = new AtomicReference<Boolean>();
-        
+
+        EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes();
+        EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes();
         EasyMock.expect(mockBundleContext.getProperty(DirectoryWatcher.DISABLE_CONFIG_SAVE)).andAnswer(
                 new IAnswer<String>() {
                     public String answer() throws Throwable {
@@ -456,7 +485,7 @@ public class ConfigInstallerTest extends TestCase {
                     }
                 }
         ).anyTimes();
-        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext);
+        EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle);
 
         ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() );
 
@@ -499,4 +528,79 @@ public class ConfigInstallerTest extends TestCase {
         EasyMock.verify(mockConfiguration, mockConfigurationAdmin, mockBundleContext);
     }
 
+    public void testSetREAD_ONLYFromFilePermissions() throws Exception
+    {
+        final File file = File.createTempFile("test", ".config");
+        try (OutputStream os = new FileOutputStream(file)) {
+            os.write("networkInterface = \"wlp3s1\"\n".getBytes("UTF-8"));
+        }
+        file.setReadOnly();
+
+        String pid = file.getName().substring(0, file.getName().indexOf(".config"));
+
+        ServiceReference<ConfigurationAdmin> sr = EasyMock.createMock(ServiceReference.class);
+
+        EasyMock.expect(mockBundleContext.getBundle()).andReturn(mockBundle).anyTimes();
+        EasyMock.expect(mockBundle.loadClass(ConfigurationAttribute.class.getName())).andReturn((Class)ConfigurationAttribute.class).anyTimes();
+        EasyMock.expect(mockBundleContext.getProperty((String) EasyMock.anyObject()))
+                .andReturn(null)
+                .anyTimes();
+
+        final Configuration cachingPersistenceConfiguration = EasyMock.createMock(Configuration.class);
+        EasyMock.expect(cachingPersistenceConfiguration.getAttributes()).andReturn(
+            Collections.singleton(ConfigurationAttribute.READ_ONLY)).anyTimes();
+        cachingPersistenceConfiguration.addAttributes(new ConfigurationAttribute[] {ConfigurationAttribute.READ_ONLY});
+        cachingPersistenceConfiguration.removeAttributes(EasyMock.anyObject(ConfigurationAttribute[].class));
+
+        final Dictionary<String, Object> cachedProps = new Hashtable<String, Object>() {
+            {
+                put("networkInterface", "wlp3s0");
+                put(DirectoryWatcher.FILENAME, file.toURI().toString());
+            }
+        };
+
+        EasyMock.expect(cachingPersistenceConfiguration.getProperties())
+                .andReturn(cachedProps)
+                .anyTimes();
+
+        EasyMock.expect(mockConfigurationAdmin.getConfiguration(pid, "?"))
+                .andReturn(cachingPersistenceConfiguration)
+                .anyTimes();
+
+        EasyMock.expect(mockConfigurationAdmin.getConfiguration(pid, null))
+                .andReturn(cachingPersistenceConfiguration)
+                .anyTimes();
+
+        final Configuration newConfiguration = EasyMock.createMock(Configuration.class);
+        EasyMock.expect(mockConfigurationAdmin.listConfigurations((String) EasyMock.anyObject()))
+                .andReturn(new Configuration[] { newConfiguration }).times(2);
+        EasyMock.expect(newConfiguration.getPid())
+                .andReturn(pid).times(2);
+
+        final Capture<Dictionary<String, Object>> props = new Capture<>();
+        cachingPersistenceConfiguration.update(EasyMock.capture(props));
+        EasyMock.expectLastCall()
+                .andAnswer(new IAnswer<Boolean>() {
+                    @Override
+                    public Boolean answer() throws Throwable {
+
+                        cachedProps.put("networkInterface", props.getValue().get("networkInterface"));
+
+                        return Boolean.TRUE;
+                   }
+               }).anyTimes();
+
+        EasyMock.replay(mockBundleContext, mockConfigurationAdmin, mockBundle, cachingPersistenceConfiguration, newConfiguration, sr);
+
+        ConfigInstaller ci = new ConfigInstaller( mockBundleContext, mockConfigurationAdmin, new FileInstall() );
+
+        ci.install(file);
+
+        file.setWritable(true);
+
+        ci.install(file);
+
+        EasyMock.verify(mockBundleContext, mockConfigurationAdmin, mockBundle, cachingPersistenceConfiguration, newConfiguration, sr);
+    }
+
 }

[felix-dev] 02/02: [FELIX-6412] Use Configuration.updateIfDifferent

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

rotty3000 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/felix-dev.git

commit 98d3533818194ce73c0dc8f8e512e46065444b95
Author: Raymond Augé <ro...@apache.org>
AuthorDate: Sun May 9 09:56:40 2021 -0400

    [FELIX-6412] Use Configuration.updateIfDifferent
    
    Signed-off-by: Raymond Augé <ro...@apache.org>
---
 .../fileinstall/internal/ConfigInstaller.java      | 31 +++++++++++++++++++++-
 .../fileinstall/internal/ConfigInstallerTest.java  | 14 +++++-----
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java b/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java
index 9b76f8a..3e11c29 100644
--- a/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java
+++ b/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java
@@ -73,6 +73,7 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
     private final Method addAttributesMethod;
     private final Method getAttributesMethod;
     private final Method removeAttributesMethod;
+    private final Method updateIfDifferentMethod;
     private final Object READ_ONLY_ATTRIBUTE_ARRAY;
     private final boolean osWin;
     private ServiceRegistration<?> registration;
@@ -86,6 +87,7 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
         Method aaMethod = null;
         Method gaMethod = null;
         Method raMethod = null;
+        Method uidMethod = null;
 
         if (this.configAdmin != null) {
             for (Method method : Configuration.class.getDeclaredMethods())
@@ -102,12 +104,17 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
                 {
                     raMethod = method;
                 }
+                else if ("updateIfDifferent".equals(method.getName()))
+                {
+                    uidMethod = method;
+                }
             }
         }
 
         this.addAttributesMethod = aaMethod;
         this.getAttributesMethod = gaMethod;
         this.removeAttributesMethod = raMethod;
+        this.updateIfDifferentMethod = uidMethod;
 
         Object roAttributesArray = null;
         if (this.addAttributesMethod != null)
@@ -393,7 +400,7 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
                         + (pid[1] == null ? "" : "-" + pid[1])
                         + "} from " + f.getName(), null);
             }
-            config.update(ht);
+            update0(config, ht);
             return true;
         }
         else
@@ -565,6 +572,28 @@ public class ConfigInstaller implements ArtifactInstaller, ConfigurationListener
         return false;
     }
 
+    void update0(final Configuration config, final Hashtable<String, Object> ht) throws IOException {
+        if (updateIfDifferentMethod != null) {
+            try {
+                updateIfDifferentMethod.invoke(config, ht);
+            }
+            catch (InvocationTargetException ite) {
+                Throwable targetException = ite.getTargetException();
+                if (targetException instanceof IOException) {
+                    throw (IOException)targetException;
+                }
+
+                throw new RuntimeException(targetException);
+            }
+            catch (Throwable t) {
+                throw new RuntimeException(t);
+            }
+        }
+        else {
+            config.update(ht);
+        }
+    }
+
     private boolean canWrite(File f) throws IOException {
         if (osWin) {
             return f.canWrite();
diff --git a/fileinstall/src/test/java/org/apache/felix/fileinstall/internal/ConfigInstallerTest.java b/fileinstall/src/test/java/org/apache/felix/fileinstall/internal/ConfigInstallerTest.java
index 6b6a12c..3eb872d 100644
--- a/fileinstall/src/test/java/org/apache/felix/fileinstall/internal/ConfigInstallerTest.java
+++ b/fileinstall/src/test/java/org/apache/felix/fileinstall/internal/ConfigInstallerTest.java
@@ -101,7 +101,7 @@ public class ConfigInstallerTest extends TestCase {
         EasyMock.expect(mockBundleContext.getProperty((String) EasyMock.anyObject()))
                 .andReturn(null)
                 .anyTimes();
-        mockConfiguration.update(EasyMock.capture(props));
+        EasyMock.expect(mockConfiguration.updateIfDifferent(EasyMock.capture(props))).andReturn(true);
         EasyMock.expectLastCall();
         EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle);
 
@@ -136,7 +136,7 @@ public class ConfigInstallerTest extends TestCase {
         EasyMock.expect(mockBundleContext.getProperty((String) EasyMock.anyObject()))
                 .andReturn(null)
                 .anyTimes();
-        mockConfiguration.update(EasyMock.capture(props));
+        EasyMock.expect(mockConfiguration.updateIfDifferent(EasyMock.capture(props))).andReturn(true);
         EasyMock.expectLastCall();
         EasyMock.replay(mockConfiguration, mockConfigurationAdmin, mockBundleContext, mockBundle);
 
@@ -248,7 +248,7 @@ public class ConfigInstallerTest extends TestCase {
                 .andReturn(mockConfiguration);
 
         ServiceReference<ConfigurationAdmin> sr = EasyMock.createMock(ServiceReference.class);
-        mockConfiguration.update(EasyMock.capture(props));
+        EasyMock.expect(mockConfiguration.updateIfDifferent(EasyMock.capture(props))).andReturn(true);
         EasyMock.expectLastCall();
 
         EasyMock.expect(mockConfigurationAdmin.getConfiguration(pid, "?"))
@@ -400,9 +400,9 @@ public class ConfigInstallerTest extends TestCase {
                 .andReturn(pid);
 
         final Capture<Dictionary<String, Object>> props = new Capture<>();
-        newConfiguration.update(EasyMock.capture(props));
+        EasyMock.expect(newConfiguration.updateIfDifferent(EasyMock.capture(props))).andReturn(true);
 
-        cachingPersistenceConfiguration.update(EasyMock.capture(props));
+        EasyMock.expect(cachingPersistenceConfiguration.updateIfDifferent(EasyMock.capture(props)));
         EasyMock.expectLastCall()
                 .andAnswer(new IAnswer<Boolean>() {
                     @Override
@@ -450,7 +450,7 @@ public class ConfigInstallerTest extends TestCase {
                 buffer.append("<Dictionary check: testkey present?>");
             }
         } );
-        mockConfiguration.update(new Hashtable<String, Object>());
+        EasyMock.expect(mockConfiguration.updateIfDifferent(new Hashtable<String, Object>())).andReturn(true);
         EasyMock.expect(mockConfigurationAdmin.listConfigurations((String) EasyMock.anyObject()))
                         .andReturn(null);
         EasyMock.expect(mockConfigurationAdmin.getConfiguration("firstcfg", "?"))
@@ -578,7 +578,7 @@ public class ConfigInstallerTest extends TestCase {
                 .andReturn(pid).times(2);
 
         final Capture<Dictionary<String, Object>> props = new Capture<>();
-        cachingPersistenceConfiguration.update(EasyMock.capture(props));
+        EasyMock.expect(cachingPersistenceConfiguration.updateIfDifferent(EasyMock.capture(props)));
         EasyMock.expectLastCall()
                 .andAnswer(new IAnswer<Boolean>() {
                     @Override