Eli Mesika has uploaded a new change for review.

Change subject: core: Underscores in tag names break tags
......................................................................

core: Underscores in tag names break tags

Problem were in the usage of the deprecated RefObject objects to pass
strings in *AutoCompleter* classes .
This was replaced with a native String usage

The signature and calls to formatValue had changed to get a
Pair<String,String> instead of two Strings encapsulated by a RefObject.

Change-Id: I8c0d48aa8ab693149fd1dcc080eddd6d2f9ba4ed
Bug-Url : https://bugzilla.redhat.com/show_bug.cgi?id=949484
Signed-off-by: Eli Mesika <[email protected]>
---
M 
backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/BaseConditionFieldAutoCompleter.java
M 
backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/DiskConditionFieldAutoCompleter.java
M 
backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/IConditionFieldAutoCompleter.java
M 
backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VdcUserConditionFieldAutoCompleter.java
M 
backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VmConditionFieldAutoCompleter.java
M 
backend/manager/modules/searchbackend/src/test/java/org/ovirt/engine/core/searchbackend/AuditLogConditionFieldAutoCompleterTest.java
6 files changed, 82 insertions(+), 93 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/66/17266/1

diff --git 
a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/BaseConditionFieldAutoCompleter.java
 
b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/BaseConditionFieldAutoCompleter.java
index a961e57..af9026b 100644
--- 
a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/BaseConditionFieldAutoCompleter.java
+++ 
b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/BaseConditionFieldAutoCompleter.java
@@ -13,11 +13,11 @@
 import org.ovirt.engine.core.common.config.Config;
 import org.ovirt.engine.core.common.config.ConfigValues;
 import org.ovirt.engine.core.common.interfaces.ITagsHandler;
+import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.compat.DateTime;
 import org.ovirt.engine.core.compat.DayOfWeek;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.compat.IntegerCompat;
-import org.ovirt.engine.core.compat.RefObject;
 import org.ovirt.engine.core.compat.Regex;
 import org.ovirt.engine.core.compat.StringFormat;
 import org.ovirt.engine.core.compat.StringHelper;
@@ -283,58 +283,55 @@
     }
 
     @Override
-    public void formatValue(String fieldName,
-            RefObject<String> relations,
-            RefObject<String> value,
-            boolean caseSensitive) {
+    public void formatValue(String fieldName, Pair<String, String> pair, 
boolean caseSensitive) {
         if (fieldName == null) {
             return;
         }
 
         if ("TIME".equals(fieldName) || "CREATIONDATE".equals(fieldName)) {
-            Date temp = DateUtils.parse(StringHelper.trim(value.argvalue, 
'\''));
+            Date temp = DateUtils.parse(StringHelper.trim(pair.getSecond(), 
'\''));
 
             DateTime result;
             if (temp == null) {
-                result = DealWithDateEnum(value.argvalue);
+                result = DealWithDateEnum(pair.getSecond());
             } else {
                 result = new DateTime(temp);
             }
 
-            if (relations.argvalue != null && relations.argvalue.equals("=")) {
-                relations.argvalue = "between";
+            if (pair.getFirst() != null && pair.getFirst().equals("=")) {
+                pair.setFirst("between");
                 DateTime nextDay = result.AddDays(1);
-                value.argvalue = StringFormat.format("'%1$s' and '%2$s'",
+                pair.setSecond(StringFormat.format("'%1$s' and '%2$s'",
                         
result.toString(DateUtils.getFormat(DateFormat.DEFAULT, DateFormat.SHORT)),
-                        
nextDay.toString(DateUtils.getFormat(DateFormat.DEFAULT, DateFormat.SHORT)));
+                        
nextDay.toString(DateUtils.getFormat(DateFormat.DEFAULT, DateFormat.SHORT))));
             } else { // ">" or "<"
                      // value.argvalue = String.format("'%1$s'", result);
-                value.argvalue = StringFormat.format("'%1$s'",
-                        
result.toString(DateUtils.getFormat(DateFormat.DEFAULT, DateFormat.SHORT)));
+                pair.setSecond(StringFormat.format("'%1$s'",
+                        
result.toString(DateUtils.getFormat(DateFormat.DEFAULT, DateFormat.SHORT))));
             }
 
         }
         else if ("TAG".equals(fieldName)) {
-            value.argvalue = value.argvalue.startsWith("N'") ? 
value.argvalue.substring(2) : value.argvalue;
-            if (relations.argvalue != null && relations.argvalue.equals("=")) {
-                relations.argvalue = "IN";
-                value.argvalue = StringHelper.trim(value.argvalue, '\'');
-                tags tag = TagsHandler.GetTagByTagName(value.argvalue);
+            pair.setSecond(pair.getSecond().startsWith("N'") ? 
pair.getSecond().substring(2) : pair.getSecond());
+            if (pair.getFirst() != null && pair.getFirst().equals("=")) {
+                pair.setFirst("IN");
+                pair.setSecond(StringHelper.trim(pair.getSecond(), '\''));
+                tags tag = TagsHandler.GetTagByTagName(pair.getSecond());
                 if (tag != null) {
-                    value.argvalue =
-                            StringFormat.format("(%1$s)", 
TagsHandler.GetTagNameAndChildrenNames(tag.gettag_id()));
+                    pair.setSecond(
+                            StringFormat.format("(%1$s)", 
TagsHandler.GetTagNameAndChildrenNames(tag.gettag_id())));
                 } else {
-                    value.argvalue = StringFormat.format("('%1$s')", 
Guid.Empty);
+                    pair.setSecond(StringFormat.format("('%1$s')", 
Guid.Empty));
                 }
-            } else if (relations.argvalue != null && 
relations.argvalue.equals("LIKE")) {
-                relations.argvalue = "IN";
-                value.argvalue = StringHelper.trim(value.argvalue, 
'\'').replace("%", "*");
+            } else if (pair.getFirst() != null && 
pair.getFirst().equals("LIKE")) {
+                pair.setFirst("IN");
+                pair.setSecond(StringHelper.trim(pair.getSecond(), 
'\'').replace("%", "*"));
 
-                String IDs = 
TagsHandler.GetTagNamesAndChildrenNamesByRegExp(value.argvalue);
+                String IDs = 
TagsHandler.GetTagNamesAndChildrenNamesByRegExp(pair.getSecond());
                 if (StringHelper.isNullOrEmpty(IDs)) {
-                    value.argvalue = StringFormat.format("('%1$s')", 
Guid.Empty);
+                    pair.setSecond(StringFormat.format("('%1$s')", 
Guid.Empty));
                 } else {
-                    value.argvalue = StringFormat.format("(%1$s)", IDs);
+                    pair.setSecond(StringFormat.format("(%1$s)", IDs));
                 }
             }
         }
@@ -371,19 +368,18 @@
     @Override
     public final String buildConditionSql(String fieldName, String 
customizedValue, String customizedRelation,
             String tableName, boolean caseSensitive) {
-        RefObject<String> tempRefObject = new 
RefObject<String>(customizedRelation);
-        RefObject<String> tempRefObject2 = new 
RefObject<String>(customizedValue);
-        formatValue(fieldName, tempRefObject, tempRefObject2, caseSensitive);
-        customizedRelation = tempRefObject.argvalue;
-        customizedValue = tempRefObject2.argvalue;
-        if (("''".equals(customizedValue) || 
"'null'".equalsIgnoreCase(customizedValue))
-                && (("=".equals(customizedRelation)) || 
("!=".equals(customizedRelation)))) {
-            String nullRelation = ("=".equals(customizedRelation)) ? "IS" : 
"IS NOT";
+        Pair<String, String> pair = new Pair<String, String>();
+        pair.setFirst(customizedRelation);
+        pair.setSecond(customizedValue);
+        formatValue(fieldName, pair, caseSensitive);
+        if (("''".equals(pair.getSecond()) || 
"'null'".equalsIgnoreCase(pair.getSecond()))
+                && (("=".equals(pair.getFirst())) || 
("!=".equals(pair.getFirst())))) {
+            String nullRelation = ("=".equals(pair.getFirst())) ? "IS" : "IS 
NOT";
             return StringFormat.format("(%1$s.%2$s %3$s  NULL)", tableName,
                     getDbFieldName(fieldName), nullRelation);
         } else {
             return StringFormat.format(" %1$s.%2$s %3$s %4$s ", tableName, 
getDbFieldName(fieldName),
-                    customizedRelation, customizedValue);
+                    pair.getFirst(), pair.getSecond());
         }
     }
 }
diff --git 
a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/DiskConditionFieldAutoCompleter.java
 
b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/DiskConditionFieldAutoCompleter.java
index a001edd..fc047e5 100644
--- 
a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/DiskConditionFieldAutoCompleter.java
+++ 
b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/DiskConditionFieldAutoCompleter.java
@@ -5,7 +5,7 @@
 
 import org.ovirt.engine.core.common.businessentities.Disk.DiskStorageType;
 import org.ovirt.engine.core.common.businessentities.ImageStatus;
-import org.ovirt.engine.core.compat.RefObject;
+import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.compat.StringFormat;
 import org.ovirt.engine.core.compat.StringHelper;
 
@@ -87,22 +87,19 @@
         } else if ("DISK_TYPE".equals(fieldName)) {
             return new EnumValueAutoCompleter(DiskStorageType.class);
         } else if ("BOOTABLE".equals(fieldName) ||
-                   "SHAREABLE".equals(fieldName) ) {
+                "SHAREABLE".equals(fieldName)) {
             return new BitValueAutoCompleter();
         }
         return null;
     }
 
     @Override
-    public void formatValue(String fieldName,
-            RefObject<String> relations,
-            RefObject<String> value,
-            boolean caseSensitive) {
+    public void formatValue(String fieldName, Pair<String, String> pair, 
boolean caseSensitive) {
         if ("CREATIONDATE".equals(fieldName)) {
-            Date tmp = new Date(Date.parse(StringHelper.trim(value.argvalue, 
'\'')));
-            value.argvalue = StringFormat.format("'%1$s'", tmp);
+            Date tmp = new Date(Date.parse(StringHelper.trim(pair.getSecond(), 
'\'')));
+            pair.setSecond(StringFormat.format("'%1$s'", tmp));
         } else {
-            super.formatValue(fieldName, relations, value, caseSensitive);
+            super.formatValue(fieldName, pair, caseSensitive);
         }
     }
 }
diff --git 
a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/IConditionFieldAutoCompleter.java
 
b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/IConditionFieldAutoCompleter.java
index 3659e95..9d94e3c 100644
--- 
a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/IConditionFieldAutoCompleter.java
+++ 
b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/IConditionFieldAutoCompleter.java
@@ -1,6 +1,6 @@
 package org.ovirt.engine.core.searchbackend;
 
-import org.ovirt.engine.core.compat.RefObject;
+import org.ovirt.engine.core.common.utils.Pair;
 
 /**
  * An interface to be implemented by all Condition fields auto completers
@@ -26,5 +26,5 @@
             String tableName,
             boolean caseSensitive);
 
-    void formatValue(String fieldName, RefObject<String> relations, 
RefObject<String> value, boolean caseSensitive);
+    void formatValue(String fieldName, Pair<String, String> pair, boolean 
caseSensitive);
 }
diff --git 
a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VdcUserConditionFieldAutoCompleter.java
 
b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VdcUserConditionFieldAutoCompleter.java
index e3c4e50..a003b7e 100644
--- 
a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VdcUserConditionFieldAutoCompleter.java
+++ 
b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VdcUserConditionFieldAutoCompleter.java
@@ -1,7 +1,7 @@
 package org.ovirt.engine.core.searchbackend;
 
 import org.ovirt.engine.core.common.businessentities.LdapRefStatus;
-import org.ovirt.engine.core.compat.RefObject;
+import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.compat.StringHelper;
 
 public class VdcUserConditionFieldAutoCompleter extends 
BaseConditionFieldAutoCompleter {
@@ -75,17 +75,17 @@
     }
 
     @Override
-    public void formatValue(String fieldName, RefObject<String> relations, 
RefObject<String> value, boolean caseSensitive) {
+    public void formatValue(String fieldName, Pair<String, String> pair, 
boolean caseSensitive) {
         if ("STATUS".equals(fieldName)) {
-            String tmp = StringHelper.trim(value.argvalue, '\'');
-            if ("=".equals(relations.argvalue) && "1".equals(tmp)) {
-                relations.argvalue = ">=";
+            String tmp = StringHelper.trim(pair.getSecond(), '\'');
+            if ("=".equals(pair.getFirst()) && "1".equals(tmp)) {
+                pair.setSecond(">=");
             }
-            if ("!=".equals(relations.argvalue) && "1".equals(tmp)) {
-                relations.argvalue = "<";
+            if ("!=".equals(pair.getFirst()) && "1".equals(tmp)) {
+                pair.setFirst("<");
             }
         } else {
-            super.formatValue(fieldName, relations, value, caseSensitive);
+            super.formatValue(fieldName, pair, caseSensitive);
         }
     }
 }
diff --git 
a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VmConditionFieldAutoCompleter.java
 
b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VmConditionFieldAutoCompleter.java
index 549ee74..2af5ea1 100644
--- 
a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VmConditionFieldAutoCompleter.java
+++ 
b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VmConditionFieldAutoCompleter.java
@@ -5,8 +5,8 @@
 
 import org.ovirt.engine.core.common.businessentities.VMStatus;
 import org.ovirt.engine.core.common.businessentities.VmType;
+import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.common.utils.SimpleDependecyInjector;
-import org.ovirt.engine.core.compat.RefObject;
 import org.ovirt.engine.core.compat.StringFormat;
 import org.ovirt.engine.core.compat.StringHelper;
 import org.ovirt.engine.core.compat.TimeSpan;
@@ -117,9 +117,9 @@
 
     @Override
     public IConditionValueAutoCompleter getFieldValueAutoCompleter(String 
fieldName) {
-         if ("OS".equals(fieldName)) {
-             return 
SimpleDependecyInjector.getInstance().get(OsValueAutoCompleter.class);
-         } else if ("STATUS".equals(fieldName)) {
+        if ("OS".equals(fieldName)) {
+            return 
SimpleDependecyInjector.getInstance().get(OsValueAutoCompleter.class);
+        } else if ("STATUS".equals(fieldName)) {
             return new EnumValueAutoCompleter(VMStatus.class);
         } else if ("TYPE".equals(fieldName)) {
             return new EnumValueAutoCompleter(VmType.class);
@@ -130,31 +130,28 @@
     }
 
     @Override
-    public void formatValue(String fieldName,
-            RefObject<String> relations,
-            RefObject<String> value,
-            boolean caseSensitive) {
+    public void formatValue(String fieldName, Pair<String, String> pair, 
boolean caseSensitive) {
         if ("APPS".equals(fieldName)) {
-            value.argvalue =
+            pair.setSecond(
                     
StringFormat.format(BaseConditionFieldAutoCompleter.getI18NPrefix() + 
"'%%%1$s%%'",
-                            StringHelper.trim(value.argvalue, 
'\'').replace("N'",
-                                    ""));
-            if ("=".equals(relations.argvalue)) {
-                relations.argvalue = 
BaseConditionFieldAutoCompleter.getLikeSyntax(caseSensitive);
-            } else if ("!=".equals(relations.argvalue)) {
-                relations.argvalue = "NOT " + 
BaseConditionFieldAutoCompleter.getLikeSyntax(caseSensitive);
+                            StringHelper.trim(pair.getSecond(), 
'\'').replace("N'",
+                                    "")));
+            if ("=".equals(pair.getFirst())) {
+                
pair.setFirst(BaseConditionFieldAutoCompleter.getLikeSyntax(caseSensitive));
+            } else if ("!=".equals(pair.getFirst())) {
+                pair.setFirst("NOT " + 
BaseConditionFieldAutoCompleter.getLikeSyntax(caseSensitive));
             }
         }
         else if ("UPTIME".equals(fieldName)) {
-            value.argvalue = StringHelper.trim(value.argvalue, '\'');
-            TimeSpan ts = TimeSpan.Parse(value.argvalue);
-            value.argvalue = StringFormat.format("'%1$s'", ts.TotalSeconds);
+            pair.setSecond(StringHelper.trim(pair.getSecond(), '\''));
+            TimeSpan ts = TimeSpan.Parse(pair.getSecond());
+            pair.setSecond(StringFormat.format("'%1$s'", ts.TotalSeconds));
         }
         else if ("CREATIONDATE".equals(fieldName)) {
-            Date tmp = new Date(Date.parse(StringHelper.trim(value.argvalue, 
'\'')));
-            value.argvalue = StringFormat.format("'%1$s'", tmp);
+            Date tmp = new Date(Date.parse(StringHelper.trim(pair.getSecond(), 
'\'')));
+            pair.setSecond(StringFormat.format("'%1$s'", tmp));
         } else {
-            super.formatValue(fieldName, relations, value, caseSensitive);
+            super.formatValue(fieldName, pair, caseSensitive);
         }
     }
 }
diff --git 
a/backend/manager/modules/searchbackend/src/test/java/org/ovirt/engine/core/searchbackend/AuditLogConditionFieldAutoCompleterTest.java
 
b/backend/manager/modules/searchbackend/src/test/java/org/ovirt/engine/core/searchbackend/AuditLogConditionFieldAutoCompleterTest.java
index 386646a..babdce9 100644
--- 
a/backend/manager/modules/searchbackend/src/test/java/org/ovirt/engine/core/searchbackend/AuditLogConditionFieldAutoCompleterTest.java
+++ 
b/backend/manager/modules/searchbackend/src/test/java/org/ovirt/engine/core/searchbackend/AuditLogConditionFieldAutoCompleterTest.java
@@ -11,8 +11,8 @@
 
 import org.junit.Before;
 import org.junit.Test;
+import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.compat.DateTime;
-import org.ovirt.engine.core.compat.RefObject;
 
 public class AuditLogConditionFieldAutoCompleterTest {
 
@@ -38,40 +38,39 @@
 
     @Test
     public void testformatValueWithTime() {
-        RefObject<String> rels = new RefObject<String>();
-        RefObject<String> value = new RefObject<String>();
+        Pair<String, String> pair = new Pair<String, String>();
         IConditionFieldAutoCompleter comp = new 
AuditLogConditionFieldAutoCompleter();
         Date date = new Date(72, 0, 12);
         String dateString = 
DateFormat.getDateInstance(DateFormat.SHORT).format(date);
-        value.argvalue = dateString;
-        comp.formatValue("TIME", rels, value, false);
+        pair.setSecond(dateString);
+        comp.formatValue("TIME", pair, false);
         DateFormat fmt = DateFormat.getDateTimeInstance(DateFormat.DEFAULT, 
DateFormat.SHORT);
-        assertEquals(quote(fmt.format(date)), value.argvalue);
-        value.argvalue = "1";
-        comp.formatValue("TIME", rels, value, false);
+        assertEquals(quote(fmt.format(date)), pair.getSecond());
+        pair.setSecond("1");
+        comp.formatValue("TIME", pair, false);
         // Try today
         // SimpleDateFormat fmt = new SimpleDateFormat("MM/dd/yyyy" +
         // " 23:59:59 ");
-        //Today begins at 00:00 - this is why we reset the DateTime object to 
midnight.
+        // Today begins at 00:00 - this is why we reset the DateTime object to 
midnight.
         DateTime dt = new DateTime(new Date());
         dt = dt.resetToMidnight();
-        assertEquals(quote(fmt.format(dt)), value.argvalue);
+        assertEquals(quote(fmt.format(dt)), pair.getSecond());
         // Try Yesterday
         Calendar cal = Calendar.getInstance();
         cal.setTime(new Date());
         cal.add(Calendar.DATE, -1);
-        value.argvalue = "2";
-        comp.formatValue("TIME", rels, value, false);
-        //Yesterday (as any other day) begins at 00:00 - this is why we reset 
the DateTime object to midnight.
+        pair.setSecond("2");
+        comp.formatValue("TIME", pair, false);
+        // Yesterday (as any other day) begins at 00:00 - this is why we reset 
the DateTime object to midnight.
 
         dt = new DateTime(cal.getTime());
         dt = dt.resetToMidnight();
-        assertEquals(quote(fmt.format(dt)), value.argvalue);
+        assertEquals(quote(fmt.format(dt)), pair.getSecond());
 
         // Just going to test that this works
-        value.argvalue = "Wednesday";
-        comp.formatValue("TIME", rels, value, false);
-        assertFalse("Day should be transformed to a date", 
value.argvalue.equals("Wednesday"));
+        pair.setSecond("Wednesday");
+        comp.formatValue("TIME", pair, false);
+        assertFalse("Day should be transformed to a date", 
pair.getSecond().equals("Wednesday"));
     }
 
     private String quote(String s) {


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

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

Reply via email to