Yair Zaslavsky has uploaded a new change for review.

Change subject: [WIP] tools: Refactor of EngineConfigLogic (#858769)
......................................................................

[WIP] tools: Refactor of EngineConfigLogic (#858769)

https://bugzilla.redhat.com/858769

This patch performs the following:

A. Rrefactors Engine-Config to work with
ConfigInfoProvider.

B. Introduces an updateList to hold all changes to ConfigKey
(currently , only one change when using the --set option).
The UpdateList will be persisted to DB (future revision of this
patch will introduce transaction handling)

Change-Id: Icaadd9eb23016b4866e5cfa99f919378f09fde1d
Signed-off-by: Yair Zaslavsky <[email protected]>
---
M 
backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/EngineConfigLogic.java
1 file changed, 39 insertions(+), 49 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/58/8158/1

diff --git 
a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/EngineConfigLogic.java
 
b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/EngineConfigLogic.java
index 05a1e8d..b75907d 100644
--- 
a/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/EngineConfigLogic.java
+++ 
b/backend/manager/tools/engine-config/src/main/java/org/ovirt/engine/core/config/EngineConfigLogic.java
@@ -9,6 +9,7 @@
 import java.security.InvalidParameterException;
 import java.sql.SQLException;
 import java.text.MessageFormat;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
@@ -45,6 +46,8 @@
     private ConfigKeyFactory configKeyFactory;
     private ConfigDAO configDAO;
     private EngineConfigCLIParser parser;
+    private List<ConfigKey> updateList = new ArrayList<ConfigKey>();
+    private ConfigInfoProvider configInfoProvider;
 
     public EngineConfigLogic(EngineConfigCLIParser parser) throws Exception {
         this.parser = parser;
@@ -104,7 +107,9 @@
             printKey();
             break;
         case ACTION_SET:
+            configInfoProvider = new 
ConfigInfoParserProvider(parser,getConfigDAO());
             persistValue();
+            updateKeysInDb();
             break;
         case ACTION_RELOAD:
             reloadConfigurations();
@@ -112,6 +117,14 @@
         default: // Should have already been discovered before execute
             log.debug("execute: unable to recognize action: " + actionType + 
".");
             throw new UnsupportedOperationException("Please tell me what to 
do: list? get? set? get-all? reload?");
+        }
+    }
+
+    private void updateKeysInDb() throws Exception {
+        for (ConfigKey configKey:updateList) {
+           if (updateKeyInDb(configKey)) {
+               break;
+           }
         }
     }
 
@@ -309,13 +322,9 @@
      * '--set')
      */
     private void persistValue() throws Exception {
-        String key = parser.getKey();
-        String value = parser.getValue();
-        String version = parser.getVersion();
-        if (version == null) {
-            version = startVersionDialog(key);
-        }
-
+        String key = configInfoProvider.getKey();
+        String value = configInfoProvider.getValue();
+        String version = configInfoProvider.getVersion();
         boolean sucessUpdate = persist(key, value, version);
         if (!sucessUpdate) {
             log.debug("setValue: error setting " + key + "'s value. No such 
entry"
@@ -323,44 +332,6 @@
             throw new IllegalArgumentException("Error setting " + key + "'s 
value. No such entry"
                     + (version == null ? "" : " with version " + version) + 
".");
         }
-    }
-
-    /**
-     * Is called when it is unclear which version is desired. If only one 
version exists for the given key, assumes that
-     * is the desired version, if more than one exist, prompts the user to 
choose one.
-     *
-     * @param key
-     *            The version needs to be found for this key
-     * @return A version for the given key
-     * @throws IOException
-     * @throws SQLException
-     */
-    private String startVersionDialog(String key) throws IOException, 
SQLException {
-        log.debug("starting version dialog.");
-        String version = null;
-        List<ConfigKey> keys = configDAO.getKeysForName(key);
-        if (keys.size() == 1) {
-            version = keys.get(0).getVersion();
-        } else if (keys.size() > 1) {
-            BufferedReader br = new BufferedReader(new 
InputStreamReader(System.in));
-            while (true) {
-                System.out.println("Please select a version:");
-                for (int i = 0; i < keys.size(); i++) {
-                    System.out.println(i + 1 + ". " + 
keys.get(i).getVersion());
-                }
-                int index = 0;
-                try {
-                    index = Integer.valueOf(br.readLine());
-                } catch (NumberFormatException e) {
-                    continue;
-                }
-                if (index >= 1 && index <= keys.size()) {
-                    version = keys.get(index - 1).getVersion();
-                    break;
-                }
-            }
-        }
-        return version;
     }
 
     /**
@@ -380,22 +351,30 @@
 
         try {
             configKey.safeSetValue(value);
-            res = (getConfigDAO().updateKey(configKey) == 1);
+            updateList.add(configKey);
         } catch (InvalidParameterException ipe) {
             message = (
                     "'" + value + "' is not a valid value for type " + 
configKey.getType() + ". " +
                     (configKey.getValidValues().isEmpty() ? "" : "Valid values 
are " + configKey.getValidValues())
                     );
         } catch (Exception e) {
-            message = "Error setting " + key + "'s value. " + e.getMessage();
-            log.debug("Error details: ", e);
+            message = getSetKeyExceptionMsg(e, configKey.getKey());
         }
+        throwExceptionWithMessage(message);
+        return res;
+    }
+
+    private void throwExceptionWithMessage(String message) throws 
IllegalAccessException {
         if (message != null) {
             log.debug(message);
             throw new IllegalAccessException(message);
         }
+    }
 
-        return res;
+    private String getSetKeyExceptionMsg(Exception e, String key) {
+        String message = "Error setting " + key + "'s value. " + 
e.getMessage();
+        log.debug("Error details: ", e);
+        return message;
     }
 
     public boolean persist(String key, String value) throws Exception {
@@ -431,4 +410,15 @@
     public ConfigDAO getConfigDAO() {
         return configDAO;
     }
+
+    protected boolean updateKeyInDb(ConfigKey key) throws 
IllegalAccessException {
+        boolean returnValue = true;
+        try {
+            returnValue =  getConfigDAO().updateKey(key) == 1;
+        } catch (Exception ex) {
+            String message = getSetKeyExceptionMsg(ex, key.getKey());
+            throwExceptionWithMessage(message);
+        }
+        return returnValue;
+    }
 }


--
To view, visit http://gerrit.ovirt.org/8158
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Icaadd9eb23016b4866e5cfa99f919378f09fde1d
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to