Merge branch 'bug4383'

java/com.sap.sse.common/src/com/sap/sse/common/settings/generic/converter/BooleanConverter.java

@@ -28,7 +28,14 @@ public class BooleanConverter implements ValueConverter<Boolean> {
 
     @Override
     public Boolean fromStringValue(String stringValue) {
-        return stringValue == null ? null : Boolean.valueOf(stringValue);
+        // Boolean.valueOf(stringValue) is explicitly not used here
+        // to ensure consistency to JSON value handling
+        if (Boolean.TRUE.toString().equalsIgnoreCase(stringValue)) {
+            return true;
+        } else if (Boolean.FALSE.toString().equalsIgnoreCase(stringValue)) {
+            return false;
+        }
+        throw new IllegalArgumentException("\"" + stringValue + "\" can not be parsed as boolean value");
     }
 
     @Override

java/com.sap.sse.common/src/com/sap/sse/common/settings/serializer/AbstractSettingsToJsonSerializer.java

@@ -4,6 +4,8 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 import com.sap.sse.common.Util;
 import com.sap.sse.common.settings.Settings;
@@ -28,6 +30,8 @@ import com.sap.sse.common.settings.generic.ValueSetting;
  *            JSON Array type
  */
 public abstract class AbstractSettingsToJsonSerializer<OBJECT, ARRAY> {
+    
+    private static final Logger LOG = Logger.getLogger(AbstractSettingsToJsonSerializer.class.getName());
 
     protected abstract OBJECT newOBJECT();
 
@@ -241,7 +245,13 @@ public abstract class AbstractSettingsToJsonSerializer<OBJECT, ARRAY> {
     }
 
     private <T> void deserializeValueSetting(Object jsonValue, ValueSetting<T> valueSetting) {
-        valueSetting.setValue(valueSetting.getValueConverter().fromJSONValue(jsonValue));
+        final ValueConverter<T> converter = valueSetting.getValueConverter();
+        try {
+            valueSetting.setValue(converter.fromJSONValue(jsonValue));
+        } catch(Exception e) {
+            LOG.log(Level.WARNING, "Error while converting JSON value \"" + jsonValue + "\" using converter \""
+                    + converter.getClass().getSimpleName() + "\"", e);
+        }
     }
 
     private <T> void deserializeListSetting(Object jsonValue, CollectionSetting<T> listSetting) {
@@ -295,7 +305,12 @@ public abstract class AbstractSettingsToJsonSerializer<OBJECT, ARRAY> {
         List<T> values = new ArrayList<>();
         if(jsonArray != null) {
             for (Object value : fromJsonArray(jsonArray)) {
-                values.add(converter.fromJSONValue(value));
+                try {
+                    values.add(converter.fromJSONValue(value));
+                } catch(Exception e) {
+                    LOG.log(Level.WARNING, "Error while converting JSON value \"" + value + "\" using converter \""
+                            + converter.getClass().getSimpleName() + "\"", e);
+                }
             }
         }
         return values;

java/com.sap.sse.common/src/com/sap/sse/common/settings/serializer/SettingsToStringMapSerializer.java

@@ -8,6 +8,8 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import java.util.Set;
 import java.util.TreeMap;
 
@@ -29,6 +31,8 @@ import com.sap.sse.common.settings.generic.ValueSetting;
  */
 public class SettingsToStringMapSerializer {
     
+    private static final Logger LOG = Logger.getLogger(SettingsToStringMapSerializer.class.getName());
+    
     private static final String ADDED_SUFFIX = GenericSerializableSettings.PATH_SEPARATOR + GenericSerializableSettings.ADDED_TOKEN;
     private static final String REMOVED_SUFFIX = GenericSerializableSettings.PATH_SEPARATOR + GenericSerializableSettings.REMOVED_TOKEN;
     
@@ -177,7 +181,14 @@ public class SettingsToStringMapSerializer {
             // Additional values are currently silently skipped.
             // This should only happen if somebody manipulates the data or a List setting is changed to a single setting
             // value.
-            valueSetting.setValue(valueSetting.getValueConverter().fromStringValue(iterator.next()));
+            final ValueConverter<T> valueConverter = valueSetting.getValueConverter();
+            final String stringValue = iterator.next();
+            try {
+                valueSetting.setValue(valueConverter.fromStringValue(stringValue));
+            } catch(Exception e) {
+                LOG.log(Level.WARNING, "Error while converting String value \"" + stringValue + "\" using converter \""
+                        + valueConverter.getClass().getSimpleName() + "\"", e);
+            }
         }
     }
 
@@ -197,7 +208,12 @@ public class SettingsToStringMapSerializer {
         List<T> deserializedValues = new ArrayList<>();
         if(values != null) {
             for (String stringValue : values) {
-                deserializedValues.add(valueConverter.fromStringValue(stringValue));
+                try {
+                    deserializedValues.add(valueConverter.fromStringValue(stringValue));
+                } catch(Exception e) {
+                    LOG.log(Level.WARNING, "Error while converting String value \"" + stringValue + "\" using converter \""
+                            + valueConverter.getClass().getSimpleName() + "\"", e);
+                }
             }
         }
         return deserializedValues;

java/com.sap.sse.shared.android.test/src/com/sap/sse/shared/android/test/AbstractSettingsSerializationTest.java

@@ -177,7 +177,7 @@ public abstract class AbstractSettingsSerializationTest<SOT> {
         
         @Override
         protected void addChildSettings() {
-            disallowedKey= new StringSetting("disallowed.key", this);
+            disallowedKey = new StringSetting("disallowed.key", this);
         }
     }
 

java/com.sap.sse.shared.android.test/src/com/sap/sse/shared/android/test/AbstractSettingsSerializationTestWithSettingsMap.java

@@ -2,17 +2,69 @@ package com.sap.sse.shared.android.test;
 
 import static org.junit.Assert.assertEquals;
 
+import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
 
 import org.junit.Test;
 
+import com.sap.sse.common.filter.TextOperator;
+import com.sap.sse.common.filter.TextOperator.Operators;
 import com.sap.sse.common.settings.Settings;
+import com.sap.sse.common.settings.generic.AbstractGenericSerializableSettings;
+import com.sap.sse.common.settings.generic.BooleanSetting;
+import com.sap.sse.common.settings.generic.DecimalSetting;
+import com.sap.sse.common.settings.generic.EnumSetSetting;
+import com.sap.sse.common.settings.generic.EnumSetting;
 import com.sap.sse.common.settings.generic.GenericSerializableSettings;
 import com.sap.sse.common.settings.generic.SettingsMap;
+import com.sap.sse.common.settings.generic.StringSetSetting;
+import com.sap.sse.common.settings.generic.StringSetting;
 
 public abstract class AbstractSettingsSerializationTestWithSettingsMap<SOT> extends AbstractSettingsSerializationTest<SOT> {
-
+    private static class UntypedSettings extends AbstractGenericSerializableSettings {
+        private static final long serialVersionUID = -2265305217290147424L;
+        private transient StringSetting enumSetting;
+        private transient StringSetSetting enumSetSetting;
+        private transient StringSetting numberSetting;
+        private transient StringSetting booleanSetting;
+        
+        public UntypedSettings(String enumValue, Collection<String> enumValues, String numberValue, String booleanValue) {
+            enumSetting.setValue(enumValue);
+            enumSetSetting.setValues(enumValues);
+            numberSetting.setValue(numberValue);
+            booleanSetting.setValue(booleanValue);
+        }
+        
+        @Override
+        protected void addChildSettings() {
+            enumSetting = new StringSetting("enumSetting", this);
+            enumSetSetting = new StringSetSetting("enumSetSetting", this);
+            numberSetting = new StringSetting("numberSetting", this);
+            booleanSetting = new StringSetting("booleanSetting", this);
+        }
+    }
+    
+    private static class TypedSettings extends AbstractGenericSerializableSettings {
+        private static final long serialVersionUID = -2265305217290147424L;
+        @SuppressWarnings("unused")
+        private transient EnumSetting<TextOperator.Operators> enumSetting;
+        private transient EnumSetSetting<TextOperator.Operators> enumSetSetting;
+        @SuppressWarnings("unused")
+        private transient DecimalSetting numberSetting;
+        @SuppressWarnings("unused")
+        private transient BooleanSetting booleanSetting;
+        
+        @Override
+        protected void addChildSettings() {
+            enumSetting = new EnumSetting<>("enumSetting", this, TextOperator.Operators::valueOf);
+            enumSetSetting = new EnumSetSetting<>("enumSetSetting", this, TextOperator.Operators::valueOf);
+            numberSetting = new DecimalSetting("numberSetting", this);
+            booleanSetting = new BooleanSetting("booleanSetting", this);
+        }
+    }
+    
     @Test
     public void testSettingsMapWithOneSerializableSettings() throws Exception {
         final GenericSerializableSettings settingsValues = createTestSettingsWithValues();
@@ -245,4 +297,22 @@ public abstract class AbstractSettingsSerializationTestWithSettingsMap<SOT> exte
         }
         return new SettingsMapImpl(innerMap);
     }
+    
+    /**
+     * Regression test for Bug 4383.
+     * When the serialized settings contain values that can't be converted to the setting
+     * type, those values may not break the deserialization. If a user e.g. manipulates the URL, the page may not stay
+     * empty because of an internal exception. In this case, the wrong values should be ignored instead but all other
+     * values must be deserialized.
+     */
+    @Test
+    public void testIllegalValuesInSerializedSettings() throws Exception {
+        UntypedSettings untypedSettings = new UntypedSettings("WrongEnumValue", Arrays.asList("Contains", "WrongEnumValue"), "noNumber", "noBoolean");
+        SOT serialized = serialize(untypedSettings);
+        TypedSettings deserialized = deserialize(serialized, new TypedSettings());
+        TypedSettings expected = new TypedSettings();
+        // this is the only value that will be correctly deserialized
+        expected.enumSetSetting.addValue(Operators.Contains);
+        assertEquals(expected, deserialized);
+    }
 }