You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@karaf.apache.org by gn...@apache.org on 2017/07/25 19:41:59 UTC

[1/6] karaf git commit: [KARAF-5271] Improve JDBC generic lock to better support network glitches

Repository: karaf
Updated Branches:
  refs/heads/master f5b7cce2d -> a7a627467


[KARAF-5271] Improve JDBC generic lock to better support network glitches

Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/a7a62746
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/a7a62746
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/a7a62746

Branch: refs/heads/master
Commit: a7a627467981433512149b3658145bccbac0c80e
Parents: eb99606
Author: Guillaume Nodet <gn...@apache.org>
Authored: Mon Jul 24 22:41:35 2017 +0200
Committer: Guillaume Nodet <gn...@apache.org>
Committed: Tue Jul 25 21:40:14 2017 +0200

----------------------------------------------------------------------
 .../karaf/main/lock/GenericDataSource.java      | 170 +++++++++
 .../apache/karaf/main/lock/GenericJDBCLock.java | 367 ++++++-------------
 2 files changed, 291 insertions(+), 246 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/a7a62746/main/src/main/java/org/apache/karaf/main/lock/GenericDataSource.java
----------------------------------------------------------------------
diff --git a/main/src/main/java/org/apache/karaf/main/lock/GenericDataSource.java b/main/src/main/java/org/apache/karaf/main/lock/GenericDataSource.java
new file mode 100644
index 0000000..57eaddd
--- /dev/null
+++ b/main/src/main/java/org/apache/karaf/main/lock/GenericDataSource.java
@@ -0,0 +1,170 @@
+/*
+ * 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.karaf.main.lock;
+
+import javax.sql.DataSource;
+import java.io.PrintWriter;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
+import java.util.Properties;
+import java.util.Queue;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.logging.Logger;
+
+public class GenericDataSource implements DataSource {
+
+    private static final String DRIVER_MANAGER_USER_PROPERTY = "user";
+    private static final String DRIVER_MANAGER_PASSWORD_PROPERTY = "password";
+
+    private final String driverClass;
+    private final String url;
+    private final Properties properties;
+    private final int validTimeoutMs;
+
+    private final Queue<Connection> cache;
+
+    private volatile Driver driver;
+    private volatile boolean driverClassLoaded;
+
+    public GenericDataSource(String driverClass, String url, String user, String password, boolean cache, int validTimeoutMs) {
+        this.driverClass = driverClass;
+        this.url = url;
+        this.properties = new Properties();
+        if (user != null) {
+            properties.setProperty(DRIVER_MANAGER_USER_PROPERTY, user);
+        }
+        if (password != null) {
+            properties.setProperty(DRIVER_MANAGER_PASSWORD_PROPERTY, password);
+        }
+        this.validTimeoutMs = validTimeoutMs;
+        this.cache = cache ? new ConcurrentLinkedQueue<>() : null;
+    }
+
+    private void ensureDriverLoaded() throws SQLException {
+        try {
+            if (!driverClassLoaded) {
+                synchronized (this) {
+                    if (!driverClassLoaded) {
+                        if (driverClass != null) {
+                            Class.forName(driverClass);
+                        }
+                        driverClassLoaded = true;
+                    }
+                }
+            }
+        } catch (ClassNotFoundException e) {
+            throw new SQLException("Unable to load driver class " + driverClass, e);
+        }
+    }
+
+    private Driver driver() throws SQLException {
+        if (driver == null) {
+            synchronized (this) {
+                if (driver == null) {
+                    driver = DriverManager.getDriver(url);
+                }
+            }
+        }
+        return driver;
+    }
+
+    public Connection getConnection() throws SQLException {
+        ensureDriverLoaded();
+        while (true) {
+            Connection con = cache != null ? cache.poll() : null;
+            if (con == null) {
+                con = driver().connect(url, properties);
+                if (con == null) {
+                    throw new SQLException("Invalid jdbc URL '" + url + "' for driver " + driver());
+                }
+            } else {
+                if (!con.isValid(validTimeoutMs)) {
+                    con.close();
+                    con = null;
+                }
+            }
+            if (con != null) {
+                return wrap(con);
+            }
+        }
+    }
+
+    private Connection wrap(Connection con) {
+        return (Connection) Proxy.newProxyInstance(con.getClass().getClassLoader(), new Class[] { Connection.class }, new InvocationHandler() {
+            private boolean closed = false;
+            @Override
+            public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+                if (method.getName().equals("close") && method.getParameterCount() == 0) {
+                    closed = true;
+                    if (!cache.offer(con)) {
+                        con.close();
+                    }
+                    return null;
+                } else if (method.getName().equals("isClosed") && method.getParameterCount() == 0) {
+                    return closed;
+                } else {
+                    if (closed) {
+                        throw new SQLException("Connection closed");
+                    }
+                    return method.invoke(con, args);
+                }
+            }
+        });
+    }
+
+    public Connection getConnection(String username, String password) throws SQLException {
+        throw new UnsupportedOperationException();
+    }
+
+    public PrintWriter getLogWriter() throws SQLException {
+        return null;
+    }
+
+    public void setLogWriter(PrintWriter out) throws SQLException {
+    }
+
+    public int getLoginTimeout() throws SQLException {
+        return 0;
+    }
+
+    public void setLoginTimeout(int seconds) throws SQLException {
+    }
+
+    @Override
+    public Logger getParentLogger() throws SQLFeatureNotSupportedException {
+        return null;
+    }
+
+    @Override
+    public <T> T unwrap(Class<T> iface) throws SQLException {
+        return null;
+    }
+
+    @Override
+    public boolean isWrapperFor(Class<?> iface) throws SQLException {
+        return false;
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/karaf/blob/a7a62746/main/src/main/java/org/apache/karaf/main/lock/GenericJDBCLock.java
----------------------------------------------------------------------
diff --git a/main/src/main/java/org/apache/karaf/main/lock/GenericJDBCLock.java b/main/src/main/java/org/apache/karaf/main/lock/GenericJDBCLock.java
index fe49e92..0e485e9 100644
--- a/main/src/main/java/org/apache/karaf/main/lock/GenericJDBCLock.java
+++ b/main/src/main/java/org/apache/karaf/main/lock/GenericJDBCLock.java
@@ -19,7 +19,6 @@
 package org.apache.karaf.main.lock;
 
 import java.sql.Connection;
-import java.sql.DriverManager;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
@@ -31,6 +30,8 @@ import java.util.logging.Logger;
 import org.apache.karaf.main.ConfigProperties;
 import org.apache.karaf.main.util.BootstrapLogManager;
 
+import javax.sql.DataSource;
+
 /**
  * <p>This class is the base class used to provide a master/slave configuration for
  * a given set of active karaf instances using JDBC. </p>
@@ -127,22 +128,26 @@ public class GenericJDBCLock implements Lock {
 
     final Logger LOG = Logger.getLogger(this.getClass().getName());
 
-    public static final String PROPERTY_LOCK_URL               = "karaf.lock.jdbc.url";
-    public static final String PROPERTY_LOCK_JDBC_DRIVER       = "karaf.lock.jdbc.driver";
-    public static final String PROPERTY_LOCK_JDBC_USER         = "karaf.lock.jdbc.user";
-    public static final String PROPERTY_LOCK_JDBC_PASSWORD     = "karaf.lock.jdbc.password";
-    public static final String PROPERTY_LOCK_JDBC_TABLE        = "karaf.lock.jdbc.table";
-    public static final String PROPERTY_LOCK_JDBC_TABLE_ID     = "karaf.lock.jdbc.table_id";
-    public static final String PROPERTY_LOCK_JDBC_CLUSTERNAME  = "karaf.lock.jdbc.clustername";
+    public static final String PROPERTY_LOCK_URL                = "karaf.lock.jdbc.url";
+    public static final String PROPERTY_LOCK_JDBC_DRIVER        = "karaf.lock.jdbc.driver";
+    public static final String PROPERTY_LOCK_JDBC_USER          = "karaf.lock.jdbc.user";
+    public static final String PROPERTY_LOCK_JDBC_PASSWORD      = "karaf.lock.jdbc.password";
+    public static final String PROPERTY_LOCK_JDBC_TABLE         = "karaf.lock.jdbc.table";
+    public static final String PROPERTY_LOCK_JDBC_TABLE_ID      = "karaf.lock.jdbc.table_id";
+    public static final String PROPERTY_LOCK_JDBC_CLUSTERNAME   = "karaf.lock.jdbc.clustername";
+    public static final String PROPERTY_LOCK_JDBC_CACHE         = "karaf.lock.jdbc.cache";
+    public static final String PROPERTY_LOCK_JDBC_VALID_TIMEOUT = "karaf.lock.jdbc.valid_timeout";
 
     public static final String DEFAULT_PASSWORD = "";
     public static final String DEFAULT_USER = "";
     public static final String DEFAULT_TABLE = "KARAF_LOCK";
     public static final String DEFAULT_TABLE_ID = "KARAF_NODE_ID";
     public static final String DEFAULT_CLUSTERNAME = "karaf";
+    public static final String DEFAULT_CACHE = "true";
+    public static final String DEFAULT_VALID_TIMEOUT = "0";
 
     final GenericStatements statements;
-    Connection lockConnection;
+    DataSource dataSource;
     String url;
     String driver;
     String user;
@@ -179,6 +184,13 @@ public class GenericJDBCLock implements Lock {
 
         this.statements = createStatements();
 
+        String url = this.url;
+        if (url.toLowerCase().startsWith("jdbc:derby")) {
+            url = (url.toLowerCase().contains("create=true")) ? url : url + ";create=true";
+        }
+        boolean cacheEnabled = Boolean.parseBoolean(props.getProperty(PROPERTY_LOCK_JDBC_CACHE, DEFAULT_CACHE));
+        int validTimeout = Integer.parseInt(props.getProperty(PROPERTY_LOCK_JDBC_VALID_TIMEOUT, DEFAULT_VALID_TIMEOUT));
+        this.dataSource = new GenericDataSource(driver, url, user, password, cacheEnabled, validTimeout);
         init();
     }
     
@@ -188,15 +200,16 @@ public class GenericJDBCLock implements Lock {
      * @return An instance of a JDBCStatement object.
      */
     GenericStatements createStatements() {
-        GenericStatements statements = new GenericStatements(table, table_id, clusterName);
-        return statements;
+        return new GenericStatements(table, table_id, clusterName);
     }
     
     void init() {
         try {
             createDatabase();
-            createSchema();
-            generateUniqueId();
+            try (Connection connection = getConnection()) {
+                createSchema(connection);
+                generateUniqueId(connection);
+            }
         } catch (Exception e) {
             LOG.log(Level.SEVERE, "Error occured while attempting to obtain connection", e);
         }
@@ -209,43 +222,28 @@ public class GenericJDBCLock implements Lock {
     /**
      * This method is called to check and create the required schemas that are used by this instance.
      */
-    void createSchema() {
-        if (schemaExists()) {
-            return;
-        }
-
-        String[] createStatments = this.statements.getLockCreateSchemaStatements(System.currentTimeMillis());
-        Statement statement = null;
-        Connection connection = null;
-
+    void createSchema(Connection connection) {
         try {
-            connection = getConnection();
-            statement = connection.createStatement();
-            connection.setAutoCommit(false);
-
-            for (String stmt : createStatments) {
-                LOG.info("Executing statement: " + stmt);
-                statement.execute(stmt);
+            if (schemaExists(connection)) {
+                return;
             }
+            try (Statement statement = connection.createStatement()) {
+                connection.setAutoCommit(false);
 
-            connection.commit();
-        } catch (Exception e) {
-            LOG.log(Level.SEVERE, "Could not create schema", e );
-            try {
-                // Rollback transaction if and only if there was a failure...
-                if (connection != null)
-                    connection.rollback();
-            } catch (Exception ie) {
-                // Do nothing....
-            }
-        } finally {
-            closeSafely(statement);
-            try {
-                // Reset the auto commit to true
+                String[] createStatements = this.statements.getLockCreateSchemaStatements(System.currentTimeMillis());
+                for (String stmt : createStatements) {
+                    LOG.info("Executing statement: " + stmt);
+                    statement.execute(stmt);
+                }
+
+                connection.commit();
                 connection.setAutoCommit(true);
-            } catch (SQLException ignored) {
-                LOG.log(Level.FINE, "Exception while setting the connection auto commit", ignored);
+            } catch (Exception e) {
+                connection.rollback();
+                throw e;
             }
+        } catch (Exception e) {
+            LOG.log(Level.SEVERE, "Could not create schema", e );
         }
     }
 
@@ -254,9 +252,9 @@ public class GenericJDBCLock implements Lock {
      *
      * @return True, if the schemas are available else false.
      */
-    boolean schemaExists() {
-        return schemaExist(this.statements.getLockTableName())
-                && schemaExist(this.statements.getLockIdTableName());
+    boolean schemaExists(Connection connection) {
+        return schemaExist(connection, this.statements.getLockTableName())
+                && schemaExist(connection, this.statements.getLockIdTableName());
     }
 
     /**
@@ -265,16 +263,12 @@ public class GenericJDBCLock implements Lock {
      * @param tableName The name of the table to determine if it exists.
      * @return True, if the table exists else false.
      */
-    private boolean schemaExist(String tableName) {
-        ResultSet rs = null;
+    private boolean schemaExist(Connection connection, String tableName) {
         boolean schemaExists = false;
-        try {
-            rs = getConnection().getMetaData().getTables(null, null, tableName, new String[] {"TABLE"});
+        try (ResultSet rs = connection.getMetaData().getTables(null, null, tableName, new String[] {"TABLE"})) {
             schemaExists = rs.next();
         } catch (Exception ignore) {
             LOG.log(Level.SEVERE, "Error testing for db table", ignore);
-        } finally {
-            closeSafely(rs);
         }
         return schemaExists;
     }
@@ -283,23 +277,18 @@ public class GenericJDBCLock implements Lock {
      * This method will generate a unique id for this instance that is part of an active set of instances.
      * This method uses a simple algorithm to insure that the id will be unique for all cases.
      */
-    void generateUniqueId() {
-        boolean uniqueIdSet = false;
+    void generateUniqueId(Connection connection) {
         String selectString = this.statements.getLockIdSelectStatement();
-        PreparedStatement selectStatement = null, updateStatement = null;
-        try {
-            selectStatement = getConnection().prepareStatement(selectString);
+        try (PreparedStatement selectStatement = connection.prepareStatement(selectString)) {
 
-            // This loop can only be performed for so long and the chances that this will be 
+            boolean uniqueIdSet = false;
+            // This loop can only be performed for so long and the chances that this will be
             // looping for more than a few times is unlikely since there will always be at
             // least one instance that is successful.
             while (!uniqueIdSet) {
 
-                ResultSet rs = null;
-
-                try {
-                    // Get the current ID from the karaf ids table
-                    rs = selectStatement.executeQuery();
+                // Get the current ID from the karaf ids table
+                try (ResultSet rs = selectStatement.executeQuery()) {
 
                     // Check if we were able to retrieve the result...
                     if (rs.next()) {
@@ -308,127 +297,38 @@ public class GenericJDBCLock implements Lock {
                         
 						String updateString = this.statements.getLockIdUpdateIdStatement(currentId + 1, currentId);
 
-						updateStatement = getConnection().prepareStatement(updateString);
-                        
-                        int count = updateStatement.executeUpdate();
-                        
-                        // Set the uniqueId if and only if is it greater that zero
-                        uniqueId = ( uniqueIdSet = count > 0 ) ? currentId + 1 : 0;
-                        
-                        if (count > 1) {
-                            LOG.severe("OOPS there are more than one row within the table ids...");
+						try (PreparedStatement updateStatement = connection.prepareStatement(updateString)) {
+
+                            int count = updateStatement.executeUpdate();
+
+                            // Set the uniqueId if and only if is it greater that zero
+                            uniqueId = (uniqueIdSet = count > 0) ? currentId + 1 : 0;
+
+                            if (count > 1) {
+                                LOG.severe("OOPS there are more than one row within the table ids...");
+                            }
                         }
                     } else {
                         LOG.severe("No rows were found....");
                     }
                 } catch (SQLException e) {
                     LOG.log(Level.SEVERE, "Received an SQL exception while processing result set", e);
-                } finally {
-                    this.closeSafely(rs);
                 }
             }
         } catch (SQLException e) {
             LOG.log(Level.SEVERE, "Received an SQL exception while generating a prepate statement", e);
-        } catch (Exception e) {
-            LOG.log(Level.SEVERE, "Received an exception while trying to get a reference to a connection", e);
-        } finally {
-            closeSafely(selectStatement);
         }
         LOG.info("INSTANCE unique id: " + uniqueId);
     }
     
     /**
-     * This method is called to determine if this instance JDBC connection is
-     * still connected.
-     *
-     * @return True, if the connection is still connected else false.
-     * @throws SQLException If an SQL error occurs while checking if the lock is connected to the database.
-     */
-    boolean isConnected() throws SQLException {
-        return lockConnection != null && !lockConnection.isClosed();
-    }
-
-    /**
-     * This method is called to safely close a Statement.
-     *
-     * @param preparedStatement The statement to be closed.
-     */
-    void closeSafely(Statement preparedStatement) {
-        if (preparedStatement != null) {
-            try {
-                preparedStatement.close();
-            } catch (SQLException e) {
-                LOG.log(Level.SEVERE, "Failed to close statement", e);
-            }
-        }
-    }
-
-    /**
-     * This method is called to safely close a ResultSet instance.
-     *
-     * @param rs The result set to be closed.
-     */
-    void closeSafely(ResultSet rs) {
-        if (rs != null) {
-            try {
-                rs.close();
-            } catch (SQLException e) {
-                LOG.log(Level.SEVERE, "Error occured while releasing ResultSet", e);
-            }
-        }
-    }
-
-    /**
      * This method will return an active connection for this given jdbc driver.
      *
      * @return The JDBC connection instance
      * @throws Exception If the JDBC connection can't be retrieved.
      */
     protected Connection getConnection() throws Exception {
-        if (!isConnected()) {
-            lockConnection = createConnection(driver, url, user, password);
-        }
-
-        return lockConnection;
-    }
-
-    /**
-     * Create a new JDBC connection.
-     *
-     * @param driver The fully qualified driver class name.
-     * @param url The database connection URL.
-     * @param username The username for the database.
-     * @param password  The password for the database.
-     * @return a new JDBC connection.
-     * @throws Exception If the JDBC connection can't be created.
-     */
-    protected Connection createConnection(String driver, String url, String username, String password) throws Exception {
-        if (url.toLowerCase().startsWith("jdbc:derby")) {
-            url = (url.toLowerCase().contains("create=true")) ? url : url + ";create=true";
-        }
-
-        try {
-            return doCreateConnection(driver, url, username, password);
-        } catch (Exception e) {
-            LOG.log(Level.SEVERE, "Error occured while setting up JDBC connection", e);
-            throw e;
-        }
-    }
-
-    /**
-     * This method could be used to inject a mock JDBC connection for testing purposes.
-     *
-     * @param driver The fully qualified driver class name.
-     * @param url The database connection URL.
-     * @param username The username for the database.
-     * @param password The password for the database.
-     * @return a new JDBC connection.
-     * @throws ClassNotFoundException If the JDBC driver class is not found.
-     * @throws SQLException If the JDBC connection can't be created.
-     */
-    protected Connection doCreateConnection(String driver, String url, String username, String password) throws ClassNotFoundException, SQLException {
-        Class.forName(driver);
-        return DriverManager.getConnection(url, username, password);
+        return dataSource.getConnection();
     }
 
     /**
@@ -441,61 +341,60 @@ public class GenericJDBCLock implements Lock {
      * @see org.apache.karaf.main.lock.Lock#lock()
      */
     public boolean lock() throws Exception {
-       
-        // Try to acquire/update the lock state
-        boolean lockAquired = acquireLock(statements.getLockUpdateIdStatement(uniqueId, ++state, lock_delay, uniqueId));
-        
-        if (!lockAquired) {
-            
-            String lockSelectStatement = statements.getLockSelectStatement();
-            PreparedStatement statement = null;
-            ResultSet rs = null;
-            
-            try {
-                statement = getConnection().prepareStatement(lockSelectStatement);
-                // Get the current master id and compare with information that we have locally....
-                rs = statement.executeQuery();
-                
-                if (rs.next()) {
-                    int currentId = statements.getIdFromLockSelectStatement(rs); // The current master unique id or 0
-                    int currentState = statements.getStateFromLockSelectStatement(rs); // The current master state or whatever
-                    
-                    if (this.currentId == currentId) {
-                        // It is the same instance that locked the table
-                        if (this.currentState == currentState) {
-                            // Its state has not been updated....
-                            if ( (this.currentStateTime + this.currentLockDelay + this.currentLockDelay) < System.currentTimeMillis() ) {
-                                // The state was not been updated for more than twice the lock_delay value of the current master...
-                                // Try to steal the lock....
-                                lockAquired = acquireLock(statements.getLockUpdateIdStatementToStealLock(uniqueId, state, lock_delay, currentId, currentState));
+        try (Connection connection = getConnection()) {
+            // Try to acquire/update the lock state
+            boolean lockAquired = acquireLock(connection, statements.getLockUpdateIdStatement(uniqueId, ++state, lock_delay, uniqueId));
+
+            if (!lockAquired) {
+
+                String lockSelectStatement = statements.getLockSelectStatement();
+
+                try (PreparedStatement statement = connection.prepareStatement(lockSelectStatement)) {
+                    // Get the current master id and compare with information that we have locally....
+                    try (ResultSet rs = statement.executeQuery()) {
+
+                        if (rs.next()) {
+                            int currentId = statements.getIdFromLockSelectStatement(rs); // The current master unique id or 0
+                            int currentState = statements.getStateFromLockSelectStatement(rs); // The current master state or whatever
+
+                            if (this.currentId == currentId) {
+                                // It is the same instance that locked the table
+                                if (this.currentState == currentState) {
+                                    // Its state has not been updated....
+                                    if ((this.currentStateTime + this.currentLockDelay + this.currentLockDelay) < System.currentTimeMillis()) {
+                                        // The state was not been updated for more than twice the lock_delay value of the current master...
+                                        // Try to steal the lock....
+                                        lockAquired = acquireLock(connection, statements.getLockUpdateIdStatementToStealLock(uniqueId, state, lock_delay, currentId, currentState));
+                                    }
+                                } else {
+                                    // Set the current time to be used to determine if we can
+                                    // try to steal the lock later...
+                                    this.currentStateTime = System.currentTimeMillis();
+                                    this.currentState = currentState;
+                                }
+                            } else {
+                                // This is a different currentId that is being used...
+                                // at this time, it does not matter if the new master id is zero we can try to acquire it
+                                // during the next lock call...
+                                this.currentId = currentId;
+                                this.currentState = currentState;
+                                // Update the current state time since this is a new lock service...
+                                this.currentStateTime = System.currentTimeMillis();
+                                // Get the lock delay value which is specific to the current master...
+                                this.currentLockDelay = statements.getLockDelayFromLockSelectStatement(rs);
                             }
-                        } else {
-                            // Set the current time to be used to determine if we can 
-                            // try to steal the lock later...
-                            this.currentStateTime = System.currentTimeMillis();
-                            this.currentState = currentState;
                         }
-                    } else {
-                        // This is a different currentId that is being used...
-                        // at this time, it does not matter if the new master id is zero we can try to acquire it
-                        // during the next lock call...
-                        this.currentId = currentId;
-                        this.currentState = currentState;
-                        // Update the current state time since this is a new lock service...
-                        this.currentStateTime = System.currentTimeMillis();
-                        // Get the lock delay value which is specific to the current master...
-                        this.currentLockDelay = statements.getLockDelayFromLockSelectStatement(rs);
                     }
+                } catch (Exception e) {
+                    LOG.log(Level.SEVERE, "Unable to determine if the lock was obtain", e);
                 }
-            } catch( Exception e ) {
-                LOG.log(Level.SEVERE, "Unable to determine if the lock was obtain", e);
-            } finally {
-                closeSafely(statement);
-                closeSafely(rs);
             }
+
+            return lockAquired;
+        } catch (Exception e) {
+            LOG.log(Level.SEVERE, "Error while trying to obtain the lock", e);
+            return false;
         }
-        
-        return lockAquired;
     }
 
     /**
@@ -506,22 +405,15 @@ public class GenericJDBCLock implements Lock {
      * @param lockUpdateIdStatement The sql statement used to execute the update.
      * @return True, if the row was updated else false.
      */
-    private boolean acquireLock(String lockUpdateIdStatement) {
-        PreparedStatement preparedStatement = null;
-        boolean lockAquired = false;
-        
-        try {
-            preparedStatement = getConnection().prepareStatement(lockUpdateIdStatement);
+    private boolean acquireLock(Connection connection, String lockUpdateIdStatement) {
+        try (PreparedStatement preparedStatement = connection.prepareStatement(lockUpdateIdStatement)) {
             // This will only update the row that contains the ID of 0 or curId
-            lockAquired = preparedStatement.executeUpdate() > 0;
+            return preparedStatement.executeUpdate() > 0;
         } catch (Exception e) {
             // Do we want to display this message everytime???
             LOG.log(Level.WARNING, "Failed to acquire database lock", e);
-        } finally {
-            closeSafely(preparedStatement);
+            return false;
         }
-        
-        return lockAquired;
     }
 
     /**
@@ -532,27 +424,15 @@ public class GenericJDBCLock implements Lock {
      * @see org.apache.karaf.main.lock.Lock#release()
      */
     public void release() throws Exception {
-        if (isConnected()) {
+        try (Connection connection = getConnection()) {
             String lockResetIdStatement = statements.getLockResetIdStatement(uniqueId);
-            PreparedStatement preparedStatement = null;
-            
-            try {
-                preparedStatement = getConnection().prepareStatement(lockResetIdStatement);
+            try (PreparedStatement preparedStatement = connection.prepareStatement(lockResetIdStatement)) {
                 // This statement will set the ID to 0 and allow others to steal the lock...
                 preparedStatement.executeUpdate();
-            } catch (SQLException e) {
-                LOG.log(Level.SEVERE, "Exception while rollbacking the connection on release", e);
-            } finally {
-                closeSafely(preparedStatement);
-                try {
-                    getConnection().close();
-                } catch (SQLException ignored) {
-                    LOG.log(Level.FINE, "Exception while closing connection on release", ignored);
-                }
             }
+        } catch (SQLException e) {
+            LOG.log(Level.SEVERE, "Exception while releasing lock", e);
         }
-        
-        lockConnection = null;
     }
 
     /**
@@ -565,11 +445,6 @@ public class GenericJDBCLock implements Lock {
      *
      */
     public boolean isAlive() throws Exception {
-        if (!isConnected()) { 
-            LOG.severe("Lost lock!");
-            return false; 
-        }
-
         return lock();
     }
 


[4/6] karaf git commit: Rename BundleInstallSupport#saveState() to saveDigraph()

Posted by gn...@apache.org.
Rename BundleInstallSupport#saveState() to saveDigraph()

Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/1c21e522
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/1c21e522
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/1c21e522

Branch: refs/heads/master
Commit: 1c21e522bebb35b25321e06528f595b79670f527
Parents: c4a4de2
Author: Guillaume Nodet <gn...@apache.org>
Authored: Mon Jul 24 22:47:45 2017 +0200
Committer: Guillaume Nodet <gn...@apache.org>
Committed: Tue Jul 25 21:40:14 2017 +0200

----------------------------------------------------------------------
 .../java/org/apache/karaf/features/internal/osgi/Activator.java  | 4 ++--
 .../karaf/features/internal/service/BundleInstallSupport.java    | 2 +-
 .../features/internal/service/BundleInstallSupportImpl.java      | 4 ++--
 .../karaf/features/internal/service/FeaturesServiceImpl.java     | 2 +-
 .../karaf/features/internal/service/StaticInstallSupport.java    | 2 +-
 .../karaf/features/internal/service/FeaturesServiceImplTest.java | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/1c21e522/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java b/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java
index b3ec0a1..876ec8e 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java
@@ -139,7 +139,7 @@ public class Activator extends BaseActivator {
                     systemBundleContext,
                     configInstaller,
                     dg);
-        register(RegionDigraphPersistence.class, () -> installSupport.saveState());
+        register(RegionDigraphPersistence.class, () -> installSupport.saveDigraph());
 
         FeatureRepoFinder featureFinder = new FeatureRepoFinder();
         register(ManagedService.class, featureFinder, FeatureRepoFinder.getServiceProperties());
@@ -302,7 +302,7 @@ public class Activator extends BaseActivator {
             featuresService = null;
         }
         if (installSupport != null) {
-            installSupport.saveState();
+            installSupport.saveDigraph();
         }
     }
 

http://git-wip-us.apache.org/repos/asf/karaf/blob/1c21e522/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupport.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupport.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupport.java
index 6c26d14..8b0ffc4 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupport.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupport.java
@@ -58,7 +58,7 @@ public interface BundleInstallSupport {
                         Map<String, Set<Long>> bundles)
         throws BundleException, InvalidSyntaxException;
 
-    void saveState();
+    void saveDigraph();
 
     RegionDigraph getDiGraphCopy() throws BundleException;
     

http://git-wip-us.apache.org/repos/asf/karaf/blob/1c21e522/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupportImpl.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupportImpl.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupportImpl.java
index 562425f..ae9a406 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupportImpl.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupportImpl.java
@@ -279,10 +279,10 @@ public class BundleInstallSupportImpl implements BundleInstallSupport {
     }
     
     /* (non-Javadoc)
-     * @see org.apache.karaf.features.internal.service.Regions#saveState()
+     * @see org.apache.karaf.features.internal.service.Regions#saveDigraph()
      */
     @Override
-    public void saveState() {
+    public void saveDigraph() {
         DigraphHelper.saveDigraph(getDataFile(DigraphHelper.DIGRAPH_FILE), digraph);
     }
     

http://git-wip-us.apache.org/repos/asf/karaf/blob/1c21e522/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
index 9600f11..e5a8b54 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
@@ -218,7 +218,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
                     state.bundleChecksums.clear();
                 }
                 storage.save(state);
-                installSupport.saveState();
+                installSupport.saveDigraph();
             }
         } catch (IOException e) {
             LOGGER.warn("Error saving FeaturesService state", e);

http://git-wip-us.apache.org/repos/asf/karaf/blob/1c21e522/features/core/src/main/java/org/apache/karaf/features/internal/service/StaticInstallSupport.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/StaticInstallSupport.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/StaticInstallSupport.java
index 2936b46..eaecccb 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/StaticInstallSupport.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/StaticInstallSupport.java
@@ -74,7 +74,7 @@ public abstract class StaticInstallSupport implements BundleInstallSupport {
     }
 
     @Override
-    public void saveState() {
+    public void saveDigraph() {
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/karaf/blob/1c21e522/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java b/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
index b6dc70a..01ad34b 100644
--- a/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
@@ -226,7 +226,7 @@ public class FeaturesServiceImplTest extends TestBase {
             }
 
             @Override
-            protected void saveState() {
+            protected void saveDigraph() {
 
             }
 


[6/6] karaf git commit: Remove outdate and useless comments

Posted by gn...@apache.org.
Remove outdate and useless comments

Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/ef830854
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/ef830854
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/ef830854

Branch: refs/heads/master
Commit: ef830854949021b2fb3965e53c6e05108674aa87
Parents: 1c21e52
Author: Guillaume Nodet <gn...@apache.org>
Authored: Mon Jul 24 22:48:38 2017 +0200
Committer: Guillaume Nodet <gn...@apache.org>
Committed: Tue Jul 25 21:40:14 2017 +0200

----------------------------------------------------------------------
 .../service/BundleInstallSupportImpl.java       | 34 --------------------
 1 file changed, 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/ef830854/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupportImpl.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupportImpl.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupportImpl.java
index ae9a406..e4cfa62 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupportImpl.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/BundleInstallSupportImpl.java
@@ -86,10 +86,6 @@ public class BundleInstallSupportImpl implements BundleInstallSupport {
         this.digraph = digraph;
     }
     
-    /* (non-Javadoc)
-     * @see org.apache.karaf.features.internal.service.Regions#print(java.lang.String, boolean)
-     */
-    @Override
     public void print(String message, boolean verbose) {
         LOGGER.info(message);
         if (verbose) {
@@ -97,9 +93,6 @@ public class BundleInstallSupportImpl implements BundleInstallSupport {
         }
     }
 
-    /* (non-Javadoc)
-     * @see org.apache.karaf.features.internal.service.Regions#refreshPackages(java.util.Collection)
-     */
     @Override
     public void refreshPackages(Collection<Bundle> bundles) throws InterruptedException {
         final CountDownLatch latch = new CountDownLatch(1);
@@ -113,9 +106,6 @@ public class BundleInstallSupportImpl implements BundleInstallSupport {
         latch.await();
     }
 
-    /* (non-Javadoc)
-     * @see org.apache.karaf.features.internal.service.Regions#installBundle(java.lang.String, java.lang.String, java.io.InputStream)
-     */
     @Override
     public Bundle installBundle(String region, String uri, InputStream is) throws BundleException {
         if (FeaturesService.ROOT_REGION.equals(region)) {
@@ -125,9 +115,6 @@ public class BundleInstallSupportImpl implements BundleInstallSupport {
         }
     }
 
-    /* (non-Javadoc)
-     * @see org.apache.karaf.features.internal.service.Regions#updateBundle(org.osgi.framework.Bundle, java.lang.String, java.io.InputStream)
-     */
     @Override
     public void updateBundle(Bundle bundle, String uri, InputStream is) throws BundleException {
         // We need to wrap the bundle to insert a Bundle-UpdateLocation header
@@ -140,9 +127,6 @@ public class BundleInstallSupportImpl implements BundleInstallSupport {
         }
     }
 
-    /* (non-Javadoc)
-     * @see org.apache.karaf.features.internal.service.Regions#uninstall(org.osgi.framework.Bundle)
-     */
     @Override
     public void uninstall(Bundle bundle) throws BundleException {
         bundle.uninstall();
@@ -155,25 +139,16 @@ public class BundleInstallSupportImpl implements BundleInstallSupport {
         }
     }
 
-    /* (non-Javadoc)
-     * @see org.apache.karaf.features.internal.service.Regions#stopBundle(org.osgi.framework.Bundle, int)
-     */
     @Override
     public void stopBundle(Bundle bundle, int options) throws BundleException {
         bundle.stop(options);
     }
 
-    /* (non-Javadoc)
-     * @see org.apache.karaf.features.internal.service.Regions#setBundleStartLevel(org.osgi.framework.Bundle, int)
-     */
     @Override
     public void setBundleStartLevel(Bundle bundle, int startLevel) {
         bundle.adapt(BundleStartLevel.class).setStartLevel(startLevel);
     }
 
-    /* (non-Javadoc)
-     * @see org.apache.karaf.features.internal.service.Regions#resolveBundles(java.util.Set, java.util.Map, java.util.Map)
-     */
     @Override
     public void resolveBundles(Set<Bundle> bundles, final Map<Resource, List<Wire>> wiring, Map<Resource, Bundle> resToBnd) {
         // Make sure it's only used for us
@@ -240,9 +215,6 @@ public class BundleInstallSupportImpl implements BundleInstallSupport {
         }
     }
 
-    /* (non-Javadoc)
-     * @see org.apache.karaf.features.internal.service.Regions#replaceDigraph(java.util.Map, java.util.Map)
-     */
     @Override
     public void replaceDigraph(Map<String, Map<String, Map<String, Set<String>>>> policies, Map<String, Set<Long>> bundles) throws BundleException, InvalidSyntaxException {
         RegionDigraph temp = digraph.copy();
@@ -278,17 +250,11 @@ public class BundleInstallSupportImpl implements BundleInstallSupport {
         digraph.replace(temp);
     }
     
-    /* (non-Javadoc)
-     * @see org.apache.karaf.features.internal.service.Regions#saveDigraph()
-     */
     @Override
     public void saveDigraph() {
         DigraphHelper.saveDigraph(getDataFile(DigraphHelper.DIGRAPH_FILE), digraph);
     }
     
-    /* (non-Javadoc)
-     * @see org.apache.karaf.features.internal.service.Regions#getGraph()
-     */
     @Override
     public RegionDigraph getDiGraphCopy() throws BundleException {
         return digraph.copy();


[3/6] karaf git commit: Make FeaturesServiceConfig immutable

Posted by gn...@apache.org.
Make FeaturesServiceConfig immutable

Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/c4a4de24
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/c4a4de24
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/c4a4de24

Branch: refs/heads/master
Commit: c4a4de24a53849a9a6945eeaecc9b0f7bdf385c0
Parents: f5b7cce
Author: Guillaume Nodet <gn...@apache.org>
Authored: Mon Jul 24 22:42:32 2017 +0200
Committer: Guillaume Nodet <gn...@apache.org>
Committed: Tue Jul 25 21:40:14 2017 +0200

----------------------------------------------------------------------
 .../karaf/features/internal/osgi/Activator.java | 21 ++++++------
 .../internal/service/FeaturesServiceConfig.java | 34 ++++++++++++++------
 2 files changed, 35 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/c4a4de24/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java b/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java
index 5d2e996..b3ec0a1 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java
@@ -213,17 +213,16 @@ public class Activator extends BaseActivator {
     }
 
     private FeaturesServiceConfig getConfig() {
-        FeaturesServiceConfig cfg = new FeaturesServiceConfig();
-        cfg.overrides = getString("overrides", new File(System.getProperty("karaf.etc"), "overrides.properties").toURI().toString());
-        cfg.featureResolutionRange = getString("featureResolutionRange", FeaturesService.DEFAULT_FEATURE_RESOLUTION_RANGE);
-        cfg.bundleUpdateRange = getString("bundleUpdateRange", FeaturesService.DEFAULT_BUNDLE_UPDATE_RANGE);
-        cfg.updateSnapshots = getString("updateSnapshots", FeaturesService.DEFAULT_UPDATE_SNAPSHOTS);
-        cfg.downloadThreads = getInt("downloadThreads", FeaturesService.DEFAULT_DOWNLOAD_THREADS);
-        cfg.scheduleDelay = getLong("scheduleDelay", FeaturesService.DEFAULT_SCHEDULE_DELAY);
-        cfg.scheduleMaxRun = getInt("scheduleMaxRun", FeaturesService.DEFAULT_SCHEDULE_MAX_RUN);
-        cfg.blacklisted = getString("blacklisted", new File(System.getProperty("karaf.etc"), "blacklisted.properties").toURI().toString());
-        cfg.serviceRequirements = getString("serviceRequirements", FeaturesService.SERVICE_REQUIREMENTS_DEFAULT);
-        return cfg;
+        return new FeaturesServiceConfig(
+            getString("overrides", new File(System.getProperty("karaf.etc"), "overrides.properties").toURI().toString()),
+            getString("featureResolutionRange", FeaturesService.DEFAULT_FEATURE_RESOLUTION_RANGE),
+            getString("bundleUpdateRange", FeaturesService.DEFAULT_BUNDLE_UPDATE_RANGE),
+            getString("updateSnapshots", FeaturesService.DEFAULT_UPDATE_SNAPSHOTS),
+            getInt("downloadThreads", FeaturesService.DEFAULT_DOWNLOAD_THREADS),
+            getLong("scheduleDelay", FeaturesService.DEFAULT_SCHEDULE_DELAY),
+            getInt("scheduleMaxRun", FeaturesService.DEFAULT_SCHEDULE_MAX_RUN),
+            getString("blacklisted", new File(System.getProperty("karaf.etc"), "blacklisted.properties").toURI().toString()),
+            getString("serviceRequirements", FeaturesService.SERVICE_REQUIREMENTS_DEFAULT));
     }
 
     private StateStorage createStateStorage() {

http://git-wip-us.apache.org/repos/asf/karaf/blob/c4a4de24/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceConfig.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceConfig.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceConfig.java
index 45362e3..68f957c 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceConfig.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceConfig.java
@@ -18,20 +18,20 @@ package org.apache.karaf.features.internal.service;
 
 public class FeaturesServiceConfig {
 
-    public String overrides;
+    public final String overrides;
     
     /**
      * Range to use when a version is specified on a feature dependency.
      * The default is {@link org.apache.karaf.features.FeaturesService#DEFAULT_FEATURE_RESOLUTION_RANGE}
      */
-    public String featureResolutionRange;
+    public final String featureResolutionRange;
     
     /**
      * Range to use when verifying if a bundle should be updated or
      * new bundle installed.
      * The default is {@link org.apache.karaf.features.FeaturesService#DEFAULT_BUNDLE_UPDATE_RANGE}
      */
-    public String bundleUpdateRange;
+    public final String bundleUpdateRange;
     
     /**
      * Use CRC to check snapshot bundles and update them if changed.
@@ -40,18 +40,34 @@ public class FeaturesServiceConfig {
      * - always : always update snapshots
      * - crc : use CRC to detect changes
      */
-    public String updateSnapshots;
+    public final String updateSnapshots;
     
-    public int downloadThreads = 1;
+    public final int downloadThreads;
     
-    public long scheduleDelay;
+    public final long scheduleDelay;
     
-    public int scheduleMaxRun;
+    public final int scheduleMaxRun;
     
     /**
      * Service requirements enforcement
      */
-    public String serviceRequirements;
+    public final String serviceRequirements;
     
-    public String blacklisted;
+    public final String blacklisted;
+
+    public FeaturesServiceConfig() {
+        this(null, null, null, null, 1, 0, 0, null, null);
+    }
+
+    public FeaturesServiceConfig(String overrides, String featureResolutionRange, String bundleUpdateRange, String updateSnapshots, int downloadThreads, long scheduleDelay, int scheduleMaxRun, String blacklisted, String serviceRequirements) {
+        this.overrides = overrides;
+        this.featureResolutionRange = featureResolutionRange;
+        this.bundleUpdateRange = bundleUpdateRange;
+        this.updateSnapshots = updateSnapshots;
+        this.downloadThreads = downloadThreads;
+        this.scheduleDelay = scheduleDelay;
+        this.scheduleMaxRun = scheduleMaxRun;
+        this.blacklisted = blacklisted;
+        this.serviceRequirements = serviceRequirements;
+    }
 }


[2/6] karaf git commit: Make the Deployer.Callback class sufficient

Posted by gn...@apache.org.
Make the Deployer.Callback class sufficient

Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/bc9adda4
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/bc9adda4
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/bc9adda4

Branch: refs/heads/master
Commit: bc9adda43e33c8c1a8f83c14944844427f0a384d
Parents: ef83085
Author: Guillaume Nodet <gn...@apache.org>
Authored: Mon Jul 24 22:51:27 2017 +0200
Committer: Guillaume Nodet <gn...@apache.org>
Committed: Tue Jul 25 21:40:14 2017 +0200

----------------------------------------------------------------------
 .../features/internal/service/Deployer.java     |  59 +++++---
 .../internal/service/FeaturesServiceImpl.java   |  63 +++++++-
 .../features/internal/service/DeployerTest.java | 149 ++++++++++++-------
 .../apache/karaf/profile/assembly/Builder.java  |   2 +-
 .../org/apache/karaf/tooling/VerifyMojo.java    |   2 +-
 5 files changed, 195 insertions(+), 80 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/bc9adda4/features/core/src/main/java/org/apache/karaf/features/internal/service/Deployer.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/Deployer.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/Deployer.java
index 4996b9b..5f7b105 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/Deployer.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/Deployer.java
@@ -61,6 +61,7 @@ import org.eclipse.equinox.region.RegionDigraph;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleException;
 import org.osgi.framework.Constants;
+import org.osgi.framework.InvalidSyntaxException;
 import org.osgi.framework.ServiceReference;
 import org.osgi.framework.Version;
 import org.osgi.framework.namespace.BundleNamespace;
@@ -114,6 +115,20 @@ public class Deployer {
         void persistResolveRequest(DeploymentRequest request) throws IOException;
         void callListeners(DeploymentEvent deployEvent);
         void callListeners(FeatureEvent featureEvent);
+
+        Bundle installBundle(String region, String uri, InputStream is) throws BundleException;
+        void updateBundle(Bundle bundle, String uri, InputStream is) throws BundleException;
+        void uninstall(Bundle bundle) throws BundleException;
+        void startBundle(Bundle bundle) throws BundleException;
+        void stopBundle(Bundle bundle, int options) throws BundleException;
+        void setBundleStartLevel(Bundle bundle, int startLevel);
+        void resolveBundles(Set<Bundle> bundles, Map<Resource, List<Wire>> wiring,
+                            Map<Resource, Bundle> resToBnd);
+        void refreshPackages(Collection<Bundle> bundles) throws InterruptedException;
+        void replaceDigraph(Map<String, Map<String, Map<String, Set<String>>>> policies,
+                            Map<String, Set<Long>> bundles) throws BundleException, InvalidSyntaxException;
+        void installConfigs(Feature feature) throws IOException, InvalidSyntaxException;
+        void installLibraries(Feature feature) throws IOException;
     }
 
     @SuppressWarnings("serial")
@@ -184,13 +199,11 @@ public class Deployer {
 
     private final DownloadManager manager;
     private final Resolver resolver;
-    private final BundleInstallSupport installSupport;
     private final DeployCallback callback;
 
-    public Deployer(DownloadManager manager, Resolver resolver, BundleInstallSupport installSupport, DeployCallback callback) {
+    public Deployer(DownloadManager manager, Resolver resolver, DeployCallback callback) {
         this.manager = manager;
         this.resolver = resolver;
-        this.installSupport = installSupport;
         this.callback = callback;
     }
 
@@ -357,7 +370,7 @@ public class Deployer {
             Set<? extends Resource> unmanaged = apply(flatten(unmanagedBundles), adapt(BundleRevision.class));
             Set<Resource> requested = new HashSet<>();
             // Gather bundles required by a feature
-            if (resolver != null && resolver.getWiring() != null) {
+            if (resolver.getWiring() != null) {
                 for (List<Wire> wires : resolver.getWiring().values()) {
                     for (Wire wire : wires) {
                         if (features.contains(wire.getRequirer()) && unmanaged.contains(wire.getProvider())) {
@@ -369,7 +382,7 @@ public class Deployer {
             // Now, we know which bundles are completely unmanaged
             unmanaged.removeAll(requested);
             // Check if bundles have wires from really unmanaged bundles
-            if (resolver != null && resolver.getWiring() != null) {
+            if (resolver.getWiring() != null) {
                 for (List<Wire> wires : resolver.getWiring().values()) {
                     for (Wire wire : wires) {
                         if (requested.contains(wire.getProvider()) && unmanaged.contains(wire.getRequirer())) {
@@ -597,14 +610,14 @@ public class Deployer {
                     dstate.bundles.values(),
                     Collections.emptyMap(),
                     Collections.emptyMap());
-            installSupport.stopBundle(serviceBundle, STOP_TRANSIENT);
+            callback.stopBundle(serviceBundle, STOP_TRANSIENT);
             try (
                     InputStream is = getBundleInputStream(resource, providers)
             ) {
-                installSupport.updateBundle(serviceBundle, uri, is);
+                callback.updateBundle(serviceBundle, uri, is);
             }
-            installSupport.refreshPackages(toRefresh.keySet());
-            installSupport.startBundle(serviceBundle);
+            callback.refreshPackages(toRefresh.keySet());
+            callback.startBundle(serviceBundle);
             return;
         }
 
@@ -630,7 +643,7 @@ public class Deployer {
                     print("  " + bundle.getSymbolicName() + "/" + bundle.getVersion(), verbose);
                     // If the bundle start level will be changed, stop it persistently to
                     // avoid a restart when the start level is actually changed
-                    installSupport.stopBundle(bundle, toUpdateStartLevel.containsKey(bundle) ? 0 : STOP_TRANSIENT);
+                    callback.stopBundle(bundle, toUpdateStartLevel.containsKey(bundle) ? 0 : STOP_TRANSIENT);
                     toStop.remove(bundle);
                 }
             }
@@ -652,7 +665,7 @@ public class Deployer {
                 Deployer.RegionDeployment regionDeployment = entry.getValue();
                 for (Bundle bundle : regionDeployment.toDelete) {
                     print("  " + bundle.getSymbolicName() + "/" + bundle.getVersion(), verbose);
-                    installSupport.uninstall(bundle);
+                    callback.uninstall(bundle);
                     removeFromMapSet(managedBundles, name, bundle.getBundleId());
                 }
             }
@@ -692,7 +705,7 @@ public class Deployer {
                 }
             }
             // Apply all changes
-            installSupport.replaceDigraph(policies, bundles);
+            callback.replaceDigraph(policies, bundles);
         }
 
 
@@ -716,7 +729,7 @@ public class Deployer {
                     try (
                             InputStream is = getBundleInputStream(resource, providers)
                     ) {
-                        installSupport.updateBundle(bundle, uri, is);
+                        callback.updateBundle(bundle, uri, is);
                     }
                     toStart.add(bundle);
                 }
@@ -728,7 +741,7 @@ public class Deployer {
         for (Map.Entry<Bundle, Integer> entry : toUpdateStartLevel.entrySet()) {
             Bundle bundle = entry.getKey();
             int sl = entry.getValue();
-            bundle.adapt(BundleStartLevel.class).setStartLevel(sl);
+            callback.setBundleStartLevel(bundle, sl);
         }
         //
         // Install bundles
@@ -753,7 +766,7 @@ public class Deployer {
                     try (
                             ChecksumUtils.CRCInputStream is = new ChecksumUtils.CRCInputStream(getBundleInputStream(resource, providers))
                     ) {
-                        bundle = installSupport.installBundle(name, uri, is);
+                        bundle = callback.installBundle(name, uri, is);
                         crc = is.getCRC();
                     }
                     addToMapSet(managedBundles, name, bundle.getBundleId());
@@ -808,14 +821,14 @@ public class Deployer {
             Set<String> featureIds = flatten(newFeatures);
             for (Feature feature : dstate.features.values()) {
                 if (featureIds.contains(feature.getId())) {
-                    installSupport.installConfigs(feature);
-                    installSupport.installLibraries(feature);
+                    callback.installConfigs(feature);
+                    callback.installLibraries(feature);
                 }
                 for (Conditional cond : feature.getConditional()) {
                     Feature condFeature = cond.asFeature();
                     if (featureIds.contains(condFeature.getId())) {
-                        installSupport.installConfigs(condFeature);
-                        installSupport.installLibraries(condFeature);
+                        callback.installConfigs(condFeature);
+                        callback.installLibraries(condFeature);
                     }
                 }
             }
@@ -838,7 +851,7 @@ public class Deployer {
                     List<Bundle> bs = getBundlesToStop(toStop);
                     for (Bundle bundle : bs) {
                         print("  " + bundle.getSymbolicName() + "/" + bundle.getVersion(), verbose);
-                        installSupport.stopBundle(bundle, STOP_TRANSIENT);
+                        callback.stopBundle(bundle, STOP_TRANSIENT);
                         toStop.remove(bundle);
                         toStart.add(bundle);
                     }
@@ -855,7 +868,7 @@ public class Deployer {
                 if (serviceBundle != null && toRefresh.containsKey(serviceBundle)) {
                     ensureAllClassesLoaded(serviceBundle);
                 }
-                installSupport.refreshPackages(toRefresh.keySet());
+                callback.refreshPackages(toRefresh.keySet());
 
             }
         }
@@ -865,7 +878,7 @@ public class Deployer {
         toResolve.addAll(toRefresh.keySet());
         removeBundlesInState(toResolve, UNINSTALLED);
         callback.callListeners(DeploymentEvent.BUNDLES_INSTALLED);
-        installSupport.resolveBundles(toResolve, resolver.getWiring(), deployment.resToBnd);
+        callback.resolveBundles(toResolve, resolver.getWiring(), deployment.resToBnd);
         callback.callListeners(DeploymentEvent.BUNDLES_RESOLVED);
 
         // Compute bundles to start
@@ -879,7 +892,7 @@ public class Deployer {
                 for (Bundle bundle : bs) {
                     print("  " + bundle.getSymbolicName() + "/" + bundle.getVersion(), verbose);
                     try {
-                        installSupport.startBundle(bundle);
+                        callback.startBundle(bundle);
                     } catch (BundleException e) {
                         exceptions.add(e);
                     }

http://git-wip-us.apache.org/repos/asf/karaf/blob/bc9adda4/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
index e5a8b54..353d40d 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
@@ -20,6 +20,7 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -70,7 +71,12 @@ import org.apache.karaf.util.collections.CopyOnWriteArrayIdentityList;
 import org.eclipse.equinox.region.RegionDigraph;
 import org.ops4j.pax.url.mvn.MavenResolver;
 import org.ops4j.pax.url.mvn.MavenResolvers;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleException;
+import org.osgi.framework.InvalidSyntaxException;
 import org.osgi.framework.Version;
+import org.osgi.resource.Resource;
+import org.osgi.resource.Wire;
 import org.osgi.service.cm.Configuration;
 import org.osgi.service.cm.ConfigurationAdmin;
 import org.osgi.service.resolver.Resolver;
@@ -1045,7 +1051,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
                 try {
                     Deployer.DeploymentState dstate = getDeploymentState(state);
                     Deployer.DeploymentRequest request = getDeploymentRequest(requirements, stateChanges, options, outputFile);
-                    new Deployer(manager, this.resolver, this.installSupport, this).deploy(dstate, request);
+                    new Deployer(manager, this.resolver, this).deploy(dstate, request);
                     break;
                 } catch (Deployer.PartialDeploymentException e) {
                     if (!prereqs.containsAll(e.getMissing())) {
@@ -1104,6 +1110,61 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         writeResolve(request.requirements, request.options);
     }
 
+    @Override
+    public void refreshPackages(Collection<Bundle> bundles) throws InterruptedException {
+        installSupport.refreshPackages(bundles);
+    }
+
+    @Override
+    public Bundle installBundle(String region, String uri, InputStream is) throws BundleException {
+        return installSupport.installBundle(region, uri, is);
+    }
+
+    @Override
+    public void updateBundle(Bundle bundle, String uri, InputStream is) throws BundleException {
+        installSupport.updateBundle(bundle, uri, is);
+    }
+
+    @Override
+    public void uninstall(Bundle bundle) throws BundleException {
+        installSupport.uninstall(bundle);
+    }
+
+    @Override
+    public void startBundle(Bundle bundle) throws BundleException {
+        installSupport.startBundle(bundle);
+    }
+
+    @Override
+    public void stopBundle(Bundle bundle, int options) throws BundleException {
+        installSupport.stopBundle(bundle, options);
+    }
+
+    @Override
+    public void setBundleStartLevel(Bundle bundle, int startLevel) {
+        installSupport.setBundleStartLevel(bundle, startLevel);
+    }
+
+    @Override
+    public void resolveBundles(Set<Bundle> bundles, Map<Resource, List<Wire>> wiring, Map<Resource, Bundle> resToBnd) {
+        installSupport.resolveBundles(bundles, wiring, resToBnd);
+    }
+
+    @Override
+    public void replaceDigraph(Map<String, Map<String, Map<String, Set<String>>>> policies, Map<String, Set<Long>> bundles) throws BundleException, InvalidSyntaxException {
+        installSupport.replaceDigraph(policies, bundles);
+    }
+
+    @Override
+    public void installConfigs(Feature feature) throws IOException, InvalidSyntaxException {
+        installSupport.installConfigs(feature);
+    }
+
+    @Override
+    public void installLibraries(Feature feature) throws IOException {
+        installSupport.installLibraries(feature);
+    }
+
     private Pattern getFeaturePattern(String name, String version) {
         String req = FEATURE_OSGI_REQUIREMENT_PREFIX + getFeatureRequirement(name, version);
         req = req.replace("[", "\\[");

http://git-wip-us.apache.org/repos/asf/karaf/blob/bc9adda4/features/core/src/test/java/org/apache/karaf/features/internal/service/DeployerTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/internal/service/DeployerTest.java b/features/core/src/test/java/org/apache/karaf/features/internal/service/DeployerTest.java
index bdad44c..651f88e 100644
--- a/features/core/src/test/java/org/apache/karaf/features/internal/service/DeployerTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/internal/service/DeployerTest.java
@@ -19,11 +19,13 @@ package org.apache.karaf.features.internal.service;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URL;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Hashtable;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.jar.Manifest;
@@ -37,7 +39,6 @@ import org.apache.karaf.features.FeaturesService;
 import org.apache.karaf.features.internal.resolver.Slf4jResolverLog;
 import org.apache.karaf.features.internal.support.TestBundle;
 import org.apache.karaf.features.internal.support.TestDownloadManager;
-import org.easymock.Capture;
 import org.easymock.EasyMock;
 import org.easymock.IAnswer;
 import org.easymock.IArgumentMatcher;
@@ -45,6 +46,9 @@ import org.easymock.IMocksControl;
 import org.junit.Test;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleException;
+import org.osgi.framework.InvalidSyntaxException;
+import org.osgi.resource.Resource;
+import org.osgi.resource.Wire;
 import org.osgi.service.resolver.Resolver;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -52,8 +56,6 @@ import org.slf4j.LoggerFactory;
 import static org.apache.karaf.features.FeaturesService.*;
 import static org.apache.karaf.features.internal.util.MapUtils.addToMapSet;
 import static org.easymock.EasyMock.anyInt;
-import static org.easymock.EasyMock.anyObject;
-import static org.easymock.EasyMock.expectLastCall;
 import static org.junit.Assert.fail;
 
 public class DeployerTest {
@@ -74,25 +76,24 @@ public class DeployerTest {
         Feature f101 = repo.getFeatures()[1];
 
         Deployer.DeployCallback callback = c.createMock(Deployer.DeployCallback.class);
-        BundleInstallSupport installSupport = c.createMock(BundleInstallSupportImpl.class);
-        Deployer deployer = new Deployer(manager, resolver, installSupport, callback);
+        Deployer deployer = new Deployer(manager, resolver, callback);
 
         callback.print(EasyMock.anyString(), EasyMock.anyBoolean());
         EasyMock.expectLastCall().anyTimes();
         callback.callListeners(DeploymentEvent.DEPLOYMENT_STARTED);
         EasyMock.expectLastCall();
-        installSupport.replaceDigraph(EasyMock.anyObject(),
+        callback.replaceDigraph(EasyMock.anyObject(),
                 EasyMock.anyObject());
         EasyMock.expectLastCall();
         callback.saveState(EasyMock.anyObject());
         EasyMock.expectLastCall();
-        installSupport.installConfigs(f100);
+        callback.installConfigs(f100);
         EasyMock.expectLastCall();
-        installSupport.installLibraries(f100);
+        callback.installLibraries(f100);
         EasyMock.expectLastCall();
         callback.callListeners(DeploymentEvent.BUNDLES_INSTALLED);
         EasyMock.expectLastCall();
-        installSupport.resolveBundles(EasyMock.anyObject(),
+        callback.resolveBundles(EasyMock.anyObject(),
                                 EasyMock.anyObject(),
                                 EasyMock.anyObject());
         EasyMock.expectLastCall();
@@ -104,7 +105,7 @@ public class DeployerTest {
         EasyMock.expectLastCall();
 
         Bundle bundleA = createTestBundle(1, Bundle.ACTIVE, dataDir, "a100");
-        EasyMock.expect(installSupport.installBundle(EasyMock.eq(ROOT_REGION), EasyMock.eq("a100"), EasyMock.anyObject()))
+        EasyMock.expect(callback.installBundle(EasyMock.eq(ROOT_REGION), EasyMock.eq("a100"), EasyMock.anyObject()))
                 .andReturn(bundleA);
 
         c.replay();
@@ -149,8 +150,7 @@ public class DeployerTest {
         Feature f101 = repo.getFeatures()[1];
 
         Deployer.DeployCallback callback = c.createMock(Deployer.DeployCallback.class);
-        BundleInstallSupport installSupport = c.createMock(BundleInstallSupportImpl.class);
-        Deployer deployer = new Deployer(manager, resolver, installSupport, callback);
+        Deployer deployer = new Deployer(manager, resolver, callback);
 
         final TestBundle bundleA = createTestBundle(1L, Bundle.ACTIVE, dataDir, "a100");
 
@@ -159,12 +159,12 @@ public class DeployerTest {
         callback.callListeners(DeploymentEvent.DEPLOYMENT_STARTED);
         EasyMock.expectLastCall();
 
-        installSupport.stopBundle(EasyMock.eq(bundleA), anyInt());
+        callback.stopBundle(EasyMock.eq(bundleA), anyInt());
         EasyMock.expectLastCall().andStubAnswer(() -> {
             bundleA.state = Bundle.RESOLVED;
             return null;
         });
-        installSupport.updateBundle(EasyMock.eq(bundleA), EasyMock.anyObject(), EasyMock.anyObject());
+        callback.updateBundle(EasyMock.eq(bundleA), EasyMock.anyObject(), EasyMock.anyObject());
         EasyMock.expectLastCall().andStubAnswer(new IAnswer<Object>() {
             @Override
             public Object answer() throws Throwable {
@@ -178,27 +178,27 @@ public class DeployerTest {
                 return null;
             }
         });
-        installSupport.startBundle(EasyMock.eq(bundleA));
+        callback.startBundle(EasyMock.eq(bundleA));
         EasyMock.expectLastCall();
 
-        installSupport.replaceDigraph(EasyMock.anyObject(),
+        callback.replaceDigraph(EasyMock.anyObject(),
                 EasyMock.anyObject());
         EasyMock.expectLastCall();
         callback.saveState(EasyMock.anyObject());
         EasyMock.expectLastCall();
-        installSupport.installConfigs(f101);
+        callback.installConfigs(f101);
         EasyMock.expectLastCall();
-        installSupport.installLibraries(f101);
+        callback.installLibraries(f101);
         EasyMock.expectLastCall();
         callback.callListeners(DeploymentEvent.BUNDLES_INSTALLED);
         EasyMock.expectLastCall();
-        installSupport.resolveBundles(EasyMock.eq(Collections.singleton(bundleA)),
+        callback.resolveBundles(EasyMock.eq(Collections.singleton(bundleA)),
                                 EasyMock.anyObject(),
                                 EasyMock.anyObject());
         EasyMock.expectLastCall();
         callback.callListeners(DeploymentEvent.BUNDLES_RESOLVED);
         EasyMock.expectLastCall();
-        installSupport.refreshPackages(EasyMock.eq(Collections.singleton(bundleA)));
+        callback.refreshPackages(EasyMock.eq(Collections.singleton(bundleA)));
         EasyMock.expectLastCall();
         callback.callListeners(FeatureEventMatcher.eq(new FeatureEvent(FeatureEvent.EventType.FeatureUninstalled, f100, FeaturesService.ROOT_REGION, false)));
         EasyMock.expectLastCall();
@@ -253,25 +253,24 @@ public class DeployerTest {
         Bundle serviceBundle = createTestBundle(1, Bundle.ACTIVE, dataDir, "a100");
 
         Deployer.DeployCallback callback = c.createMock(Deployer.DeployCallback.class);
-        BundleInstallSupport installSupport = c.createMock(BundleInstallSupportImpl.class);
-        Deployer deployer = new Deployer(manager, resolver, installSupport, callback);
+        Deployer deployer = new Deployer(manager, resolver, callback);
 
         callback.print(EasyMock.anyString(), EasyMock.anyBoolean());
         EasyMock.expectLastCall().anyTimes();
         callback.callListeners(DeploymentEvent.DEPLOYMENT_STARTED);
         EasyMock.expectLastCall();
-        installSupport.replaceDigraph(EasyMock.anyObject(),
+        callback.replaceDigraph(EasyMock.anyObject(),
                 EasyMock.anyObject());
         EasyMock.expectLastCall();
         callback.saveState(EasyMock.anyObject());
         EasyMock.expectLastCall();
         callback.callListeners(DeploymentEvent.BUNDLES_INSTALLED);
         EasyMock.expectLastCall();
-        installSupport.installConfigs(f1);
+        callback.installConfigs(f1);
         EasyMock.expectLastCall();
-        installSupport.installLibraries(f1);
+        callback.installLibraries(f1);
         EasyMock.expectLastCall();
-        installSupport.resolveBundles(EasyMock.anyObject(),
+        callback.resolveBundles(EasyMock.anyObject(),
                                 EasyMock.anyObject(),
                                 EasyMock.anyObject());
         EasyMock.expectLastCall();
@@ -327,27 +326,26 @@ public class DeployerTest {
         Bundle serviceBundle2 = createTestBundle(2, Bundle.ACTIVE, dataDir, "b100");
 
         Deployer.DeployCallback callback = c.createMock(Deployer.DeployCallback.class);
-        BundleInstallSupport installSupport = c.createMock(BundleInstallSupportImpl.class);
-        Deployer deployer = new Deployer(manager, resolver, installSupport, callback);
+        Deployer deployer = new Deployer(manager, resolver, callback);
 
         callback.print(EasyMock.anyString(), EasyMock.anyBoolean());
         EasyMock.expectLastCall().anyTimes();
         callback.callListeners(DeploymentEvent.DEPLOYMENT_STARTED);
         EasyMock.expectLastCall();
-        installSupport.installBundle(EasyMock.eq(ROOT_REGION), EasyMock.eq("a100"), EasyMock.anyObject());
+        callback.installBundle(EasyMock.eq(ROOT_REGION), EasyMock.eq("a100"), EasyMock.anyObject());
         EasyMock.expectLastCall().andReturn(serviceBundle1);
-        installSupport.replaceDigraph(EasyMock.anyObject(),
+        callback.replaceDigraph(EasyMock.anyObject(),
                 EasyMock.anyObject());
         EasyMock.expectLastCall();
         callback.saveState(EasyMock.anyObject());
         EasyMock.expectLastCall();
-        installSupport.installConfigs(EasyMock.anyObject());
+        callback.installConfigs(EasyMock.anyObject());
         EasyMock.expectLastCall();
-        installSupport.installLibraries(EasyMock.anyObject());
+        callback.installLibraries(EasyMock.anyObject());
         EasyMock.expectLastCall();
         callback.callListeners(DeploymentEvent.BUNDLES_INSTALLED);
         EasyMock.expectLastCall();
-        installSupport.resolveBundles(EasyMock.anyObject(),
+        callback.resolveBundles(EasyMock.anyObject(),
                 EasyMock.anyObject(),
                 EasyMock.anyObject());
         EasyMock.expectLastCall();
@@ -396,20 +394,20 @@ public class DeployerTest {
         EasyMock.expectLastCall().anyTimes();
         callback.callListeners(DeploymentEvent.DEPLOYMENT_STARTED);
         EasyMock.expectLastCall();
-        installSupport.installBundle(EasyMock.eq(ROOT_REGION), EasyMock.eq("b100"), EasyMock.anyObject());
+        callback.installBundle(EasyMock.eq(ROOT_REGION), EasyMock.eq("b100"), EasyMock.anyObject());
         EasyMock.expectLastCall().andReturn(serviceBundle2);
-        installSupport.replaceDigraph(EasyMock.anyObject(),
+        callback.replaceDigraph(EasyMock.anyObject(),
                 EasyMock.anyObject());
         EasyMock.expectLastCall();
         callback.saveState(EasyMock.anyObject());
         EasyMock.expectLastCall();
-        installSupport.installConfigs(f2);
+        callback.installConfigs(f2);
         EasyMock.expectLastCall();
-        installSupport.installLibraries(f2);
+        callback.installLibraries(f2);
         EasyMock.expectLastCall();
         callback.callListeners(DeploymentEvent.BUNDLES_INSTALLED);
         EasyMock.expectLastCall();
-        installSupport.resolveBundles(EasyMock.anyObject(),
+        callback.resolveBundles(EasyMock.anyObject(),
                 EasyMock.anyObject(),
                 EasyMock.anyObject());
         EasyMock.expectLastCall();
@@ -509,21 +507,8 @@ public class DeployerTest {
         request.stateChanges = Collections.emptyMap();
         request.updateSnaphots = UPDATE_SNAPSHOTS_NONE;
 
-        MyDeployCallback callback = new MyDeployCallback(dstate);
-        BundleInstallSupport installSupport = c.createMock(BundleInstallSupportImpl.class);
-        Capture<String> capture = Capture.newInstance();
-        installSupport.installBundle(EasyMock.anyString(), EasyMock.capture(capture), anyObject(InputStream.class));
-        EasyMock.expectLastCall().andAnswer(() -> bundles.get(capture.getValue())).atLeastOnce();
-        installSupport.installConfigs(EasyMock.anyObject());
-        EasyMock.expectLastCall().atLeastOnce();
-        installSupport.installLibraries(EasyMock.anyObject());
-        EasyMock.expectLastCall().atLeastOnce();
-        installSupport.replaceDigraph(EasyMock.anyObject(),
-                               EasyMock.anyObject());
-        expectLastCall().atLeastOnce();
-        installSupport.resolveBundles(anyObject(Set.class), anyObject(Map.class), anyObject(Map.class));
-        expectLastCall().atLeastOnce();
-        Deployer deployer = new Deployer(manager, resolver, installSupport, callback);
+        MyDeployCallback callback = new MyDeployCallback(dstate, bundles);
+        Deployer deployer = new Deployer(manager, resolver, callback);
         c.replay();
 
         for (int i = 1; i <= 4; i++) {
@@ -586,9 +571,11 @@ public class DeployerTest {
 
     private static class MyDeployCallback implements Deployer.DeployCallback {
         final Deployer.DeploymentState dstate;
+        final Map<String, Bundle> bundles;
 
-        public MyDeployCallback(Deployer.DeploymentState dstate) {
+        public MyDeployCallback(Deployer.DeploymentState dstate, Map<String, Bundle> bundles) {
             this.dstate = dstate;
+            this.bundles = bundles;
         }
 
         @Override
@@ -612,5 +599,59 @@ public class DeployerTest {
         public void callListeners(DeploymentEvent deployEvent) {
         }
 
+        @Override
+        public Bundle installBundle(String region, String uri, InputStream is) throws BundleException {
+            return bundles.get(uri);
+        }
+
+        @Override
+        public void updateBundle(Bundle bundle, String uri, InputStream is) throws BundleException {
+
+        }
+
+        @Override
+        public void uninstall(Bundle bundle) throws BundleException {
+
+        }
+
+        @Override
+        public void startBundle(Bundle bundle) throws BundleException {
+
+        }
+
+        @Override
+        public void stopBundle(Bundle bundle, int options) throws BundleException {
+
+        }
+
+        @Override
+        public void setBundleStartLevel(Bundle bundle, int startLevel) {
+
+        }
+
+        @Override
+        public void resolveBundles(Set<Bundle> bundles, Map<Resource, List<Wire>> wiring, Map<Resource, Bundle> resToBnd) {
+
+        }
+
+        @Override
+        public void refreshPackages(Collection<Bundle> bundles) throws InterruptedException {
+
+        }
+
+        @Override
+        public void replaceDigraph(Map<String, Map<String, Map<String, Set<String>>>> policies, Map<String, Set<Long>> bundles) throws BundleException, InvalidSyntaxException {
+
+        }
+
+        @Override
+        public void installConfigs(Feature feature) throws IOException, InvalidSyntaxException {
+
+        }
+
+        @Override
+        public void installLibraries(Feature feature) throws IOException {
+
+        }
     }
 }

http://git-wip-us.apache.org/repos/asf/karaf/blob/bc9adda4/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java
----------------------------------------------------------------------
diff --git a/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java b/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java
index 74ef8b0..e20080c 100644
--- a/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java
+++ b/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java
@@ -1421,7 +1421,7 @@ public class Builder {
                     Collection<String> optionals) throws Exception {
         BundleRevision systemBundle = getSystemBundle();
         AssemblyDeployCallback callback = new AssemblyDeployCallback(manager, this, systemBundle, repositories);
-        Deployer deployer = new Deployer(manager, resolver, callback, callback);
+        Deployer deployer = new Deployer(manager, resolver, callback);
 
         // Install framework
         Deployer.DeploymentRequest request = createDeploymentRequest();

http://git-wip-us.apache.org/repos/asf/karaf/blob/bc9adda4/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/VerifyMojo.java
----------------------------------------------------------------------
diff --git a/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/VerifyMojo.java b/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/VerifyMojo.java
index d4595c2..5ed7522 100644
--- a/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/VerifyMojo.java
+++ b/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/VerifyMojo.java
@@ -422,7 +422,7 @@ public class VerifyMojo extends MojoSupport {
         try {
             Bundle systemBundle = getSystemBundle(getMetadata(properties, "metadata#"));
             DummyDeployCallback callback = new DummyDeployCallback(systemBundle, repositories.values());
-            Deployer deployer = new Deployer(manager, new ResolverImpl(new MavenResolverLog()), callback, callback);
+            Deployer deployer = new Deployer(manager, new ResolverImpl(new MavenResolverLog()), callback);
 
 
             // Install framework


[5/6] karaf git commit: [KARAF-5272] Enhance the features deployer so that it performs a real upgrade

Posted by gn...@apache.org.
[KARAF-5272] Enhance the features deployer so that it performs a real upgrade

Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/eb996069
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/eb996069
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/eb996069

Branch: refs/heads/master
Commit: eb9960695d978b0d6c817202e688a4e4a0f6da45
Parents: bc9adda
Author: Guillaume Nodet <gn...@apache.org>
Authored: Tue Jul 25 15:34:43 2017 +0200
Committer: Guillaume Nodet <gn...@apache.org>
Committed: Tue Jul 25 21:40:14 2017 +0200

----------------------------------------------------------------------
 deployer/features/pom.xml                       |   1 +
 .../features/FeatureDeploymentListener.java     | 216 ++++++++-----------
 .../apache/karaf/features/FeaturesService.java  |   6 +
 .../internal/service/FeaturesServiceImpl.java   |  99 +++++++--
 .../internal/service/RepositoryCache.java       |   6 +-
 .../karaf/kar/internal/KarServiceImpl.java      |   4 +-
 6 files changed, 186 insertions(+), 146 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/eb996069/deployer/features/pom.xml
----------------------------------------------------------------------
diff --git a/deployer/features/pom.xml b/deployer/features/pom.xml
index 0ff151a..d0c00bb 100644
--- a/deployer/features/pom.xml
+++ b/deployer/features/pom.xml
@@ -105,6 +105,7 @@
                             org.apache.karaf.deployer.features,
                             org.apache.karaf.deployer.features.osgi,
                             org.apache.karaf.util,
+                            org.apache.felix.utils.version
                         </Private-Package>
                     </instructions>
                 </configuration>

http://git-wip-us.apache.org/repos/asf/karaf/blob/eb996069/deployer/features/src/main/java/org/apache/karaf/deployer/features/FeatureDeploymentListener.java
----------------------------------------------------------------------
diff --git a/deployer/features/src/main/java/org/apache/karaf/deployer/features/FeatureDeploymentListener.java b/deployer/features/src/main/java/org/apache/karaf/deployer/features/FeatureDeploymentListener.java
index ab954fa..58bfc2b 100644
--- a/deployer/features/src/main/java/org/apache/karaf/deployer/features/FeatureDeploymentListener.java
+++ b/deployer/features/src/main/java/org/apache/karaf/deployer/features/FeatureDeploymentListener.java
@@ -23,25 +23,27 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.EnumSet;
 import java.util.Enumeration;
-import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
-import javax.xml.parsers.DocumentBuilder;
-import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.namespace.QName;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamReader;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.w3c.dom.Document;
 
 import org.apache.felix.fileinstall.ArtifactUrlTransformer;
+import org.apache.felix.utils.version.VersionRange;
 import org.apache.karaf.features.Feature;
 import org.apache.karaf.features.FeaturesNamespaces;
 import org.apache.karaf.features.FeaturesService;
@@ -50,9 +52,8 @@ import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleEvent;
 import org.osgi.framework.BundleListener;
-import org.xml.sax.ErrorHandler;
-import org.xml.sax.SAXException;
-import org.xml.sax.SAXParseException;
+
+import static org.apache.karaf.features.FeaturesService.ROOT_REGION;
 
 /**
  * A deployment listener able to hot deploy a feature descriptor
@@ -63,7 +64,7 @@ public class FeatureDeploymentListener implements ArtifactUrlTransformer, Bundle
 
     private final Logger logger = LoggerFactory.getLogger(FeatureDeploymentListener.class);
 
-    private DocumentBuilderFactory dbf;
+    private XMLInputFactory xif;
     private FeaturesService featuresService;
     private BundleContext bundleContext;
     private Properties properties = new Properties();
@@ -144,9 +145,9 @@ public class FeatureDeploymentListener implements ArtifactUrlTransformer, Bundle
     public boolean canHandle(File artifact) {
         try {
             if (artifact.isFile() && artifact.getName().endsWith(".xml")) {
-                Document doc = parse(artifact);
-                String name = doc.getDocumentElement().getLocalName();
-                String uri  = doc.getDocumentElement().getNamespaceURI();
+                QName qname = getRootElementName(artifact);
+                String name = qname.getLocalPart();
+                String uri  = qname.getNamespaceURI();
                 if ("features".equals(name) ) {
                 	if(isKnownFeaturesURI(uri)){
                         return true;
@@ -176,125 +177,94 @@ public class FeatureDeploymentListener implements ArtifactUrlTransformer, Bundle
         }
     }
 
-    public void bundleChanged(BundleEvent bundleEvent) {
-            Bundle bundle = bundleEvent.getBundle();
-            if (bundleEvent.getType() == BundleEvent.RESOLVED) {
-                try {
-                    List<URL> urls = new ArrayList<>();
-                    Enumeration featuresUrlEnumeration = bundle.findEntries("/META-INF/" + FEATURE_PATH + "/", "*.xml", false);
-                    while (featuresUrlEnumeration != null && featuresUrlEnumeration.hasMoreElements()) {
-                        URL url = (URL) featuresUrlEnumeration.nextElement();
-                        try {
-                            featuresService.addRepository(url.toURI());
-                            URI needRemovedRepo = null;
-                            for (Repository repo : featuresService.listRepositories()) {
-                                if (repo.getURI().equals(url.toURI())) {
-                                    Set<Feature> features = new HashSet<>(Arrays.asList(repo.getFeatures()));
-                                    Set<String> autoInstallFeatures = new HashSet<>();
-                                    for(Feature feature:features) {
-                                        if(feature.getInstall() != null && feature.getInstall().equals(Feature.DEFAULT_INSTALL_MODE)){
-                                            if (!featuresService.isInstalled(feature)) {
-                                                autoInstallFeatures.add(feature.getId());
-                                            }
-                                        }
-                                    }
-                                    if (!autoInstallFeatures.isEmpty()) {
-                                        featuresService.installFeatures(autoInstallFeatures, EnumSet.noneOf(FeaturesService.Option.class));
-                                    }
-                                } else {
-                                    //remove older out-of-data feature repo
-                                    if (repo.getURI().toString().contains(FEATURE_PATH)) {
-                                        String featureFileName = repo.getURI().toString();
-                                        featureFileName = featureFileName.substring(featureFileName.lastIndexOf('/') + 1);
-                                        String newFeatureFileName = url.toURI().toString();
-                                        newFeatureFileName = newFeatureFileName.substring(newFeatureFileName.lastIndexOf('/') + 1);
-                                        if (featureFileName.equals(newFeatureFileName)) {
-                                            needRemovedRepo = repo.getURI();
-                                        }
-                                    }
-                                }
+    public synchronized void bundleChanged(BundleEvent bundleEvent) {
+        // Only handle resolved and uninstalled events
+        if (bundleEvent.getType() != BundleEvent.RESOLVED
+                && bundleEvent.getType() != BundleEvent.UNINSTALLED) {
+            return;
+        }
+        Bundle bundle = bundleEvent.getBundle();
+        try {
+            // Remove previous informations
+            List<URI> repsToRemove = new ArrayList<>();
+            List<String> reqsToRemove = new ArrayList<>();
+            // Remove old properties
+            String prefix = "bundle." + bundle.getBundleId();
+            String countStr = (String) properties.remove(prefix + ".reps.count");
+            if (countStr != null) {
+                int count = Integer.parseInt(countStr);
+                for (int i = 0; i < count; i++) {
+                    String rep = (String) properties.remove(prefix + ".reps.item" + i);
+                    repsToRemove.add(URI.create(rep));
+                }
+            }
+            countStr = (String) properties.remove(prefix + ".reqs.count");
+            if (countStr != null) {
+                int count = Integer.parseInt(countStr);
+                for (int i = 0; i < count; i++) {
+                    String req = (String) properties.remove(prefix + ".reqs.item" + i);
+                    reqsToRemove.add(req);
+                }
+            }
+            saveProperties();
 
-                            }
-                            urls.add(url);
-                            if (needRemovedRepo != null) {
-                                featuresService.removeRepository(needRemovedRepo);
-                            }
-                        } catch (Exception e) {
-                            logger.error("Unable to install features", e);
-                        }
-                    }
-                    synchronized (this) {
-                        String prefix = bundle.getSymbolicName() + "-" + bundle.getVersion();
-                        String old = (String) properties.get(prefix + ".count");
-                        if (old != null && urls.isEmpty()) {
-                            properties.remove(prefix + ".count");
-                            saveProperties();
-                        } else if (!urls.isEmpty()) {
-                            properties.put(prefix + ".count", Integer.toString(urls.size()));
-                            for (int i = 0; i < urls.size(); i++) {
-                                properties.put(prefix + ".url." + i, urls.get(i).toExternalForm());
-                            }
-                            saveProperties();
-                        }
-                    }
-                } catch (Exception e) {
-                    logger.error("Unable to install deployed features for bundle: " + bundle.getSymbolicName() + " - " + bundle.getVersion(), e);
+            // Compute new informations
+            List<URI> repsToAdd = new ArrayList<>();
+            List<String> reqsToAdd = new ArrayList<>();
+            if (bundleEvent.getType() == BundleEvent.RESOLVED) {
+                Enumeration featuresUrlEnumeration = bundle.findEntries("/META-INF/" + FEATURE_PATH + "/", "*.xml", false);
+                while (featuresUrlEnumeration != null && featuresUrlEnumeration.hasMoreElements()) {
+                    URL url = (URL) featuresUrlEnumeration.nextElement();
+                    URI uri = url.toURI();
+                    repsToAdd.add(uri);
+                    Repository rep = featuresService.createRepository(uri);
+                    Stream.of(rep.getFeatures())
+                            .filter(f -> f.getInstall() == null || Feature.DEFAULT_INSTALL_MODE.equals(f.getInstall()))
+                            .map(f -> "feature:" + f.getName() + "/" + new VersionRange(f.getVersion(), true))
+                            .forEach(reqsToAdd::add);
                 }
-            } else if (bundleEvent.getType() == BundleEvent.UNINSTALLED) {
-                try {
-                    synchronized (this) {
-                        String prefix = bundle.getSymbolicName() + "-" + bundle.getVersion();
-                        String countStr = (String) properties.remove(prefix + ".count");
-                        if (countStr != null) {
-                            int count = Integer.parseInt(countStr);
-                            for (int i = 0; i < count; i++) {
-                                URL url = new URL((String) properties.remove(prefix + ".url." + i));
-                                for (Repository repo : featuresService.listRepositories()) {
-                                    try {
-                                        if (repo.getURI().equals(url.toURI())) {
-                                            for (Feature f : repo.getFeatures()) {
-                                                try {
-                                                    featuresService.uninstallFeature(f.getName(), f.getVersion());
-                                                } catch (Exception e) {
-                                                    logger.error("Unable to uninstall feature: " + f.getName(), e);
-                                                }
-                                            }
-                                        }
-                                    } catch (Exception e) {
-                                        logger.error("Unable to uninstall features: " + url, e);
-                                    }
-                                }
-                                try {
-                                    featuresService.removeRepository(url.toURI());
-                                } catch (URISyntaxException e) {
-                                    logger.error("Unable to remove repository: " + url, e);
-                                }
-                            }
-                            saveProperties();
-                        }
+                if (!repsToAdd.isEmpty()) {
+                    properties.put(prefix + ".reps.count", Integer.toString(repsToAdd.size()));
+                    for (int i = 0; i < repsToAdd.size(); i++) {
+                        properties.put(prefix + ".reps.item" + i, repsToAdd.get(i).toASCIIString());
+                    }
+                    properties.put(prefix + ".reqs.count", Integer.toString(reqsToAdd.size()));
+                    for (int i = 0; i < reqsToAdd.size(); i++) {
+                        properties.put(prefix + ".reqs.item" + i, reqsToAdd.get(i));
                     }
-                } catch (Exception e) {
-                    logger.error("Unable to uninstall deployed features for bundle: " + bundle.getSymbolicName() + " - " + bundle.getVersion(), e);
                 }
             }
+            saveProperties();
+
+            // Call features service
+            List<Repository> requiredRepos = Arrays.asList(featuresService.listRequiredRepositories());
+            Set<URI> requiredReposUris = requiredRepos.stream()
+                    .map(Repository::getURI).collect(Collectors.toSet());
+            requiredReposUris.removeAll(repsToRemove);
+            requiredReposUris.addAll(repsToAdd);
+
+            Map<String, Set<String>> requirements = featuresService.listRequirements();
+            requirements.get(ROOT_REGION).removeAll(reqsToRemove);
+            requirements.get(ROOT_REGION).addAll(reqsToAdd);
+
+            if (!reqsToRemove.isEmpty() || !reqsToAdd.isEmpty()) {
+                featuresService.updateReposAndRequirements(requiredReposUris, requirements, EnumSet.noneOf(FeaturesService.Option.class));
+            }
+        } catch (Exception e) {
+            logger.error("Unable to update deployed features for bundle: " + bundle.getSymbolicName() + " - " + bundle.getVersion(), e);
+        }
     }
 
-    protected Document parse(File artifact) throws Exception {
-        if (dbf == null) {
-            dbf = DocumentBuilderFactory.newInstance();
-            dbf.setNamespaceAware(true);
+    private QName getRootElementName(File artifact) throws Exception {
+        if (xif == null) {
+            xif = XMLInputFactory.newFactory();
+            xif.setProperty(XMLInputFactory.IS_NAMESPACE_AWARE, true);
+        }
+        try (InputStream is = new FileInputStream(artifact)) {
+            XMLStreamReader sr = xif.createXMLStreamReader(is);
+            sr.nextTag();
+            return sr.getName();
         }
-        DocumentBuilder db = dbf.newDocumentBuilder();
-        db.setErrorHandler(new ErrorHandler() {
-            public void warning(SAXParseException exception) throws SAXException {
-            }
-            public void error(SAXParseException exception) throws SAXException {
-            }
-            public void fatalError(SAXParseException exception) throws SAXException {
-                throw exception;
-            }
-        });
-        return db.parse(artifact);
     }
 
 }

http://git-wip-us.apache.org/repos/asf/karaf/blob/eb996069/features/core/src/main/java/org/apache/karaf/features/FeaturesService.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/FeaturesService.java b/features/core/src/main/java/org/apache/karaf/features/FeaturesService.java
index c8cdd15..29c9093 100644
--- a/features/core/src/main/java/org/apache/karaf/features/FeaturesService.java
+++ b/features/core/src/main/java/org/apache/karaf/features/FeaturesService.java
@@ -126,6 +126,12 @@ public interface FeaturesService {
 
     void updateFeaturesState(Map<String, Map<String, FeatureState>> stateChanges, EnumSet<Option> options) throws Exception;
 
+    void updateReposAndRequirements(Set<URI> repos,
+                                    Map<String, Set<String>> requirements,
+                                    EnumSet<Option> options) throws Exception;
+
+    Repository createRepository(URI uri) throws Exception;
+
     Feature[] listFeatures() throws Exception;
 
     Feature[] listRequiredFeatures() throws Exception;

http://git-wip-us.apache.org/repos/asf/karaf/blob/eb996069/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
index 353d40d..bd98364 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
@@ -42,12 +42,14 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import org.apache.felix.utils.manifest.Clause;
 import org.apache.felix.utils.version.VersionCleaner;
 import org.apache.felix.utils.version.VersionRange;
 import org.apache.felix.utils.version.VersionTable;
@@ -178,7 +180,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         // Resolve
         try {
             Map<String, Map<String, FeatureState>> stateChanges = Collections.emptyMap();
-            doProvisionInThread(requestedFeatures, stateChanges, copyState(), options);
+            doProvisionInThread(requestedFeatures, stateChanges, copyState(), getFeaturesById(), options);
         } catch (Exception e) {
             LOGGER.warn("Error updating state", e);
         }
@@ -645,6 +647,11 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         return map;
     }
 
+    protected Map<String, Feature> getFeaturesById() throws Exception {
+        return getFeatureCache().values().stream().flatMap(m -> m.values().stream())
+                .collect(Collectors.toMap(Feature::getId, Function.identity()));
+    }
+
    //
     // Installed features
     //
@@ -858,7 +865,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         }
         print("Adding features: " + join(featuresToDisplay), options.contains(Option.Verbose));
         Map<String, Map<String, FeatureState>> stateChanges = Collections.emptyMap();
-        doProvisionInThread(required, stateChanges, state, options);
+        doProvisionInThread(required, stateChanges, state, getFeaturesById(), options);
     }
 
     @Override
@@ -909,13 +916,13 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
             required.remove(region);
         }
         Map<String, Map<String, FeatureState>> stateChanges = Collections.emptyMap();
-        doProvisionInThread(required, stateChanges, state, options);
+        doProvisionInThread(required, stateChanges, state, getFeaturesById(), options);
     }
 
     @Override
     public void updateFeaturesState(Map<String, Map<String, FeatureState>> stateChanges, EnumSet<Option> options) throws Exception {
         State state = copyState();
-        doProvisionInThread(copy(state.requirements), stateChanges, state, options);
+        doProvisionInThread(copy(state.requirements), stateChanges, state, getFeaturesById(), options);
     }
 
     @Override
@@ -924,7 +931,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         Map<String, Set<String>> required = copy(state.requirements);
         add(required, requirements);
         Map<String, Map<String, FeatureState>> stateChanges = Collections.emptyMap();
-        doProvisionInThread(required, stateChanges, state, options);
+        doProvisionInThread(required, stateChanges, state, getFeaturesById(), options);
     }
 
     @Override
@@ -933,7 +940,63 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         Map<String, Set<String>> required = copy(state.requirements);
         remove(required, requirements);
         Map<String, Map<String, FeatureState>> stateChanges = Collections.emptyMap();
-        doProvisionInThread(required, stateChanges, state, options);
+        doProvisionInThread(required, stateChanges, state, getFeaturesById(), options);
+    }
+
+    @Override
+    public void updateReposAndRequirements(Set<URI> repos, Map<String, Set<String>> requirements, EnumSet<Option> options) throws Exception {
+        State stateCopy;
+        synchronized (lock) {
+            // Remove repo
+            Set<String> reps = repos.stream().map(URI::toString).collect(Collectors.toSet());
+            Set<String> toRemove = diff(state.repositories, reps);
+            Set<String> toAdd = diff(reps, state.repositories);
+            state.repositories.removeAll(toRemove);
+            state.repositories.addAll(toAdd);
+            featureCache = null;
+            for (String uri : toRemove) {
+                repositories.removeRepository(URI.create(uri));
+            }
+            for (String uri : toAdd) {
+                repositories.addRepository(createRepository(URI.create(uri)));
+            }
+            saveState();
+            stateCopy = state.copy();
+        }
+        Map<String, Map<String, FeatureState>> stateChanges = Collections.emptyMap();
+        doProvisionInThread(requirements, stateChanges, stateCopy, getFeaturesById(), options);
+    }
+
+    private <T> Set<T> diff(Set<T> s1, Set<T> s2) {
+        Set<T> s = new HashSet<>(s1);
+        s.removeAll(s2);
+        return s;
+    }
+
+    @Override
+    public Repository createRepository(URI uri) throws Exception {
+        return repositories.create(uri, true, true);
+    }
+
+    private Map<String, Feature> loadAllFeatures(Set<URI> uris) throws Exception {
+        //the outer map's key is feature name, the inner map's key is feature version
+        Map<String, Feature> map = new HashMap<>();
+        // Two phase load:
+        // * first load dependent repositories
+        Set<URI> loaded = new HashSet<>();
+        List<URI> toLoad = new ArrayList<>(uris);
+        Clause[] blacklisted = repositories.getBlacklisted();
+        while (!toLoad.isEmpty()) {
+            URI uri = toLoad.remove(0);
+            if (loaded.add(uri)) {
+                Repository repo = new RepositoryImpl(uri, blacklisted);
+                Collections.addAll(toLoad, repo.getRepositories());
+                for (Feature f : repo.getFeatures()) {
+                    map.put(f.getId(), f);
+                }
+            }
+        }
+        return map;
     }
 
     @Override
@@ -974,12 +1037,13 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
     private void doProvisionInThread(final Map<String, Set<String>> requirements,
                                     final Map<String, Map<String, FeatureState>> stateChanges,
                                     final State state,
+                                    final Map<String, Feature> featureById,
                                     final EnumSet<Option> options) throws Exception {
         try {
             final String outputFile = this.outputFile.get();
             this.outputFile.set(null);
             executor.submit(() -> {
-                doProvision(requirements, stateChanges, state, options, outputFile);
+                doProvision(requirements, stateChanges, state, featureById, options, outputFile);
                 return null;
             }).get();
         } catch (ExecutionException e) {
@@ -996,7 +1060,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         }
     }
 
-    private Deployer.DeploymentState getDeploymentState(State state) throws Exception {
+    private Deployer.DeploymentState getDeploymentState(State state, Map<String, Feature> featuresById) throws Exception {
         Deployer.DeploymentState dstate = new Deployer.DeploymentState();
         dstate.state = state;
         FrameworkInfo info = installSupport.getInfo();
@@ -1005,13 +1069,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         dstate.currentStartLevel = info.currentStartLevel;
         dstate.bundles = info.bundles;
         // Features
-        dstate.features = new HashMap<>();
-        for (Map<String, Feature> m : getFeatureCache().values()) {
-            for (Feature feature : m.values()) {
-                String id = feature.getId();
-                dstate.features.put(id, feature);
-            }
-        }
+        dstate.features = featuresById;
         RegionDigraph regionDigraph = installSupport.getDiGraphCopy();
         dstate.bundlesPerRegion = DigraphHelper.getBundlesPerRegion(regionDigraph);
         dstate.filtersPerRegion = DigraphHelper.getPolicies(regionDigraph);
@@ -1034,10 +1092,11 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
     }
 
     private void doProvision(Map<String, Set<String>> requirements,                // all requirements
-                            Map<String, Map<String, FeatureState>> stateChanges,  // features state changes
-                            State state,                                          // current state
-                            EnumSet<Option> options,                              // installation options
-                            String outputFile                                     // file to store the resolution or null
+                             Map<String, Map<String, FeatureState>> stateChanges,  // features state changes
+                             State state,                                          // current state
+                             Map<String, Feature> featuresById,                    // features by id
+                             EnumSet<Option> options,                              // installation options
+                             String outputFile                                     // file to store the resolution or null
     ) throws Exception {
 
         Dictionary<String, String> props = getMavenConfig();
@@ -1049,7 +1108,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
             Set<String> prereqs = new HashSet<>();
             while (true) {
                 try {
-                    Deployer.DeploymentState dstate = getDeploymentState(state);
+                    Deployer.DeploymentState dstate = getDeploymentState(state, featuresById);
                     Deployer.DeploymentRequest request = getDeploymentRequest(requirements, stateChanges, options, outputFile);
                     new Deployer(manager, this.resolver, this).deploy(dstate, request);
                     break;

http://git-wip-us.apache.org/repos/asf/karaf/blob/eb996069/features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryCache.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryCache.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryCache.java
index 54cf04f..f92b0bc 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryCache.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryCache.java
@@ -39,7 +39,11 @@ public class RepositoryCache {
     public RepositoryCache(String blacklisted) {
         this.blacklisted = loadBlacklist(blacklisted);
     }
-    
+
+    public Clause[] getBlacklisted() {
+        return blacklisted;
+    }
+
     private static Clause[] loadBlacklist(String blacklisted) {
         Set<String> blacklistStrings = Blacklist.loadBlacklist(blacklisted);
         return Parser.parseClauses(blacklistStrings.toArray(new String[blacklistStrings.size()]));

http://git-wip-us.apache.org/repos/asf/karaf/blob/eb996069/kar/src/main/java/org/apache/karaf/kar/internal/KarServiceImpl.java
----------------------------------------------------------------------
diff --git a/kar/src/main/java/org/apache/karaf/kar/internal/KarServiceImpl.java b/kar/src/main/java/org/apache/karaf/kar/internal/KarServiceImpl.java
index 743e74a..30f7279 100644
--- a/kar/src/main/java/org/apache/karaf/kar/internal/KarServiceImpl.java
+++ b/kar/src/main/java/org/apache/karaf/kar/internal/KarServiceImpl.java
@@ -274,7 +274,7 @@ public class KarServiceImpl implements KarService {
                 if (repository.getURI().equals(karFeatureRepoUri)) {
                     try {
                         for (Feature feature : repository.getFeatures()) {
-                            if (feature.getInstall() == null || !feature.getInstall().equals("manual")) {
+                            if (feature.getInstall() == null || Feature.DEFAULT_INSTALL_MODE.equals(feature.getInstall())) {
                                 try {
                                     LOGGER.debug("noAutoRefreshBundles is " + isNoAutoRefreshBundles());
                                     if (isNoAutoRefreshBundles()) {
@@ -430,7 +430,7 @@ public class KarServiceImpl implements KarService {
                 if (repository.getURI().equals(karFeatureRepoUri)) {
                     try {
                         for (Feature feature : repository.getFeatures()) {
-                            if (feature.getInstall() == null || !feature.getInstall().equals("manual")) {
+                            if (feature.getInstall() == null || Feature.DEFAULT_INSTALL_MODE.equals(feature.getInstall())) {
                                 try {
                                     featuresService.uninstallFeature(feature.getName(), feature.getVersion());
                                 } catch (Exception e) {