Changeset 9821 in josm for trunk/src

Timestamp:

2016-02-18T11:07:38+01:00 (9 years ago)

Author:

bastiK

Message:

fixed #12522 - Advanced preferences: display default entries consistently

Saves default preference entries to a cache file (cache/default_preferences.xml), so the list of advanced preferences is filled with all known default values consistently from the start and not gradually as you use different features during a session.

Location:

trunk/src/org/openstreetmap/josm

Files:

• Main.java (modified) (2 diffs)
• data/Preferences.java (modified) (17 diffs)
• data/preferences/AbstractSetting.java (modified) (3 diffs)
• data/preferences/PreferencesReader.java (modified) (6 diffs)
• data/preferences/Setting.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/Main.java

@@ -15 +15 @@
 import java.awt.event.WindowEvent;
 import java.io.File;
+import java.io.IOException;
 import java.lang.ref.WeakReference;
 import java.net.URI;
@@ -1098 +1099 @@
                 }
             }
+            try {
+                pref.saveDefaults();
+            } catch (IOException ex) {
+                Main.warn(tr("Failed to save default preferences."));
+            }
             worker.shutdownNow();
             ImageProvider.shutdown(true);

trunk/src/org/openstreetmap/josm/data/Preferences.java

@@ -66 +66 @@
 import org.openstreetmap.josm.tools.CheckParameterUtil;
 import org.openstreetmap.josm.tools.ColorHelper;
+import org.openstreetmap.josm.tools.FilteredCollection;
 import org.openstreetmap.josm.tools.I18n;
 import org.openstreetmap.josm.tools.MultiMap;
+import org.openstreetmap.josm.tools.Predicate;
 import org.openstreetmap.josm.tools.Utils;
 import org.xml.sax.SAXException;
@@ -102 +104 @@
     };
 
+    private static final long MAX_AGE_DEFAULT_PREFERENCES = 60 * 60 * 24 * 50; // 50 days (in seconds)
+
     /**
      * Internal storage for the preference directory.
@@ -137 +141 @@
      */
     protected final SortedMap<String, Setting<?>> defaultsMap = new TreeMap<>();
+
+    private final Predicate<Entry<String, Setting<?>>> NO_DEFAULT_SETTINGS_ENTRY = new Predicate<Entry<String, Setting<?>>>() {
+        @Override
+        public boolean evaluate(Entry<String, Setting<?>> e) {
+            return !e.getValue().equals(defaultsMap.get(e.getKey()));
+        }
+    };
 
     /**
@@ -293 +304 @@
 
     /**
-     * Returns the user preferences file (preferences.xml)
+     * Returns the user preferences file (preferences.xml).
      * @return The user preferences file (preferences.xml)
      */
@@ -301 +312 @@
 
     /**
-     * Returns the user plugin directory
+     * Returns the cache file for default preferences.
+     * @return the cache file for default preferences
+     */
+    public File getDefaultsCacheFile() {
+        return new File(getCacheDirectory(), "default_preferences.xml");
+    }
+
+    /**
+     * Returns the user plugin directory.
      * @return The user plugin directory
      */
@@ -493 +512 @@
      */
     public void save() throws IOException {
-        /* currently unused, but may help to fix configuration issues in future */
-        putInteger("josm.version", Version.getInstance().getVersion());
-
-        updateSystemProperties();
-
-        File prefFile = getPreferenceFile();
+        save(getPreferenceFile(),
+                new FilteredCollection<>(settingsMap.entrySet(), NO_DEFAULT_SETTINGS_ENTRY), false);
+    }
+
+    public void saveDefaults() throws IOException {
+        save(getDefaultsCacheFile(), defaultsMap.entrySet(), true);
+    }
+
+    public void save(File prefFile, Collection<Entry<String, Setting<?>>> settings, boolean defaults) throws IOException {
+
+        if (!defaults) {
+            /* currently unused, but may help to fix configuration issues in future */
+            putInteger("josm.version", Version.getInstance().getVersion());
+
+            updateSystemProperties();
+        }
+
         File backupFile = new File(prefFile + "_backup");
 
@@ -508 +538 @@
         try (PrintWriter out = new PrintWriter(new OutputStreamWriter(
                 new FileOutputStream(prefFile + "_tmp"), StandardCharsets.UTF_8), false)) {
-            out.print(toXML(settingsMap, false));
+            out.print(toXML(settings, false, defaults));
         }
 
@@ -546 +576 @@
         File pref = getPreferenceFile();
         PreferencesReader.validateXML(pref);
-        PreferencesReader reader = new PreferencesReader();
+        PreferencesReader reader = new PreferencesReader(false);
         reader.fromXML(pref);
         settingsMap.clear();
@@ -555 +585 @@
 
     /**
+     * Loads default preferences from default settings cache file.
+     *
+     * Discards entries older than {@link #MAX_AGE_DEFAULT_PREFERENCES}.
+     *
+     * @throws IOException if any I/O error occurs while reading the file
+     * @throws SAXException if the settings file does not contain valid XML
+     * @throws XMLStreamException if an XML error occurs while parsing the file (after validation)
+     */
+    protected void loadDefaults() throws IOException, XMLStreamException, SAXException {
+        File def = getDefaultsCacheFile();
+        PreferencesReader.validateXML(def);
+        PreferencesReader reader = new PreferencesReader(true);
+        reader.fromXML(def);
+        defaultsMap.clear();
+        long minTime = System.currentTimeMillis() / 1000 - MAX_AGE_DEFAULT_PREFERENCES;
+        for (Entry<String, Setting<?>> e : reader.getSettings().entrySet()) {
+            if (e.getValue().getTime() >= minTime) {
+                defaultsMap.put(e.getKey(), e.getValue());
+            }
+        }
+    }
+
+    /**
      * Loads preferences from XML reader.
      * @param in XML reader
@@ -560 +613 @@
      */
     public void fromXML(Reader in) throws XMLStreamException {
-        PreferencesReader reader = new PreferencesReader();
+        PreferencesReader reader = new PreferencesReader(false);
         reader.fromXML(in);
         settingsMap.clear();
@@ -629 +682 @@
             load();
             initSuccessful = true;
-        } catch (Exception e) {
+        } catch (IOException | SAXException | XMLStreamException e) {
             Main.error(e);
             File backupFile = new File(prefDir, "preferences.xml.bak");
@@ -647 +700 @@
                 Main.error(e1);
                 Main.warn(tr("Failed to initialize preferences. Failed to reset preference file to default: {0}", getPreferenceFile()));
+            }
+        }
+        File def = getDefaultsCacheFile();
+        if (def.exists()) {
+            try {
+                loadDefaults();
+            } catch (IOException | XMLStreamException | SAXException e) {
+                Main.error(e);
+                Main.warn(tr("Failed to load defaults cache file: {0}", def));
+                defaultsMap.clear();
+                if (!def.delete()) {
+                    Main.warn(tr("Failed to delete faulty defaults cache file: {0}", def));
+                }
             }
         }
@@ -880 +946 @@
         CheckParameterUtil.ensureParameterNotNull(def);
         Setting<?> oldDef = defaultsMap.get(key);
-        if (oldDef != null && oldDef.getValue() != null && def.getValue() != null && !def.equals(oldDef)) {
+        if (oldDef != null && oldDef.isNew() && oldDef.getValue() != null && def.getValue() != null && !def.equals(oldDef)) {
             Main.info("Defaults for " + key + " differ: " + def + " != " + defaultsMap.get(key));
         }
         if (def.getValue() != null || oldDef == null) {
-            defaultsMap.put(key, def.copy());
+            Setting<?> defCopy = def.copy();
+            defCopy.setTime(System.currentTimeMillis() / 1000);
+            defCopy.setNew(true);
+            defaultsMap.put(key, defCopy);
         }
         Setting<?> prop = settingsMap.get(key);
@@ -1333 +1402 @@
         private final StringBuilder b;
         private final boolean noPassword;
+        private final boolean defaults;
         private String key;
 
-        SettingToXml(StringBuilder b, boolean noPassword) {
+        SettingToXml(StringBuilder b, boolean noPassword, boolean defaults) {
             this.b = b;
             this.noPassword = noPassword;
+            this.defaults = defaults;
         }
 
         public void setKey(String key) {
             this.key = key;
+        }
+
+        private void addTime(Setting setting) {
+            if (defaults) {
+                Long time = setting.getTime();
+                if (time == null) throw new IllegalStateException();
+                b.append("' time='").append(time);
+            }
         }
 
@@ -1348 +1427 @@
             if (noPassword && "osm-server.password".equals(key))
                 return; // do not store plain password.
-            /* don't save default values */
-            if (setting.equals(defaultsMap.get(key)))
-                return;
             b.append("  <tag key='");
             b.append(XmlWriter.encode(key));
-            b.append("' value='");
-            b.append(XmlWriter.encode(setting.getValue()));
-            b.append("'/>\n");
+            addTime(setting);
+            if (setting.getValue() != null) {
+                b.append("' value='");
+                b.append(XmlWriter.encode(setting.getValue()));
+                b.append("'/>\n");
+            } else if (defaults) {
+                b.append("' xsi:nil='true'/>\n");
+            } else {
+                throw new NullPointerException();
+            }
         }
 
         @Override
         public void visit(ListSetting setting) {
-            /* don't save default values */
-            if (setting.equals(defaultsMap.get(key)))
-                return;
-            b.append("  <list key='").append(XmlWriter.encode(key)).append("'>\n");
-            for (String s : setting.getValue()) {
-                b.append("    <entry value='").append(XmlWriter.encode(s)).append("'/>\n");
-            }
-            b.append("  </list>\n");
+            b.append("  <list key='").append(XmlWriter.encode(key));
+            addTime(setting);
+            if (setting.getValue() != null) {
+                b.append("'>\n");
+                for (String s : setting.getValue()) {
+                    b.append("    <entry value='").append(XmlWriter.encode(s)).append("'/>\n");
+                }
+                b.append("  </list>\n");
+            } else if (defaults) {
+                b.append("' xsi:nil='true'/>\n");
+            } else {
+                throw new NullPointerException();
+            }
         }
 
         @Override
         public void visit(ListListSetting setting) {
-            /* don't save default values */
-            if (setting.equals(defaultsMap.get(key)))
-                return;
-            b.append("  <lists key='").append(XmlWriter.encode(key)).append("'>\n");
-            for (List<String> list : setting.getValue()) {
-                b.append("    <list>\n");
-                for (String s : list) {
-                    b.append("      <entry value='").append(XmlWriter.encode(s)).append("'/>\n");
+            b.append("  <lists key='").append(XmlWriter.encode(key));
+            addTime(setting);
+            if (setting.getValue() != null) {
+                b.append("'>\n");
+                for (List<String> list : setting.getValue()) {
+                    b.append("    <list>\n");
+                    for (String s : list) {
+                        b.append("      <entry value='").append(XmlWriter.encode(s)).append("'/>\n");
+                    }
+                    b.append("    </list>\n");
                 }
-                b.append("    </list>\n");
-            }
-            b.append("  </lists>\n");
+                b.append("  </lists>\n");
+            } else if (defaults) {
+                b.append("' xsi:nil='true'/>\n");
+            } else {
+                throw new NullPointerException();
+            }
         }
 
         @Override
         public void visit(MapListSetting setting) {
-            b.append("  <maps key='").append(XmlWriter.encode(key)).append("'>\n");
-            for (Map<String, String> struct : setting.getValue()) {
-                b.append("    <map>\n");
-                for (Entry<String, String> e : struct.entrySet()) {
-                    b.append("      <tag key='").append(XmlWriter.encode(e.getKey()))
-                     .append("' value='").append(XmlWriter.encode(e.getValue())).append("'/>\n");
+            b.append("  <maps key='").append(XmlWriter.encode(key));
+            addTime(setting);
+            if (setting.getValue() != null) {
+                b.append("'>\n");
+                for (Map<String, String> struct : setting.getValue()) {
+                    b.append("    <map>\n");
+                    for (Entry<String, String> e : struct.entrySet()) {
+                        b.append("      <tag key='").append(XmlWriter.encode(e.getKey()))
+                         .append("' value='").append(XmlWriter.encode(e.getValue())).append("'/>\n");
+                    }
+                    b.append("    </map>\n");
                 }
-                b.append("    </map>\n");
-            }
-            b.append("  </maps>\n");
+                b.append("  </maps>\n");
+            } else if (defaults) {
+                b.append("' xsi:nil='true'/>\n");
+            } else {
+                throw new NullPointerException();
+            }
         }
     }
@@ -1407 +1508 @@
      */
     public String toXML(boolean nopass) {
-        return toXML(settingsMap, nopass);
+        return toXML(settingsMap.entrySet(), nopass, false);
     }
 
@@ -1416 +1517 @@
      * @return XML
      */
-    public String toXML(Map<String, Setting<?>> settings, boolean nopass) {
-        StringBuilder b = new StringBuilder(
-                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<preferences xmlns=\"")
-                .append(Main.getXMLBase()).append("/preferences-1.0\" version=\"")
-                .append(Version.getInstance().getVersion()).append("\">\n");
-        SettingToXml toXml = new SettingToXml(b, nopass);
-        for (Entry<String, Setting<?>> e : settings.entrySet()) {
+    public String toXML(Collection<Entry<String, Setting<?>>> settings, boolean nopass, boolean defaults) {
+        String rootElement = defaults ? "preferences-defaults" : "preferences";
+        StringBuilder b = new StringBuilder("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<")
+                .append(rootElement).append(" xmlns='")
+                .append(Main.getXMLBase()).append("/preferences-1.0'");
+        if (defaults) {
+            b.append(" xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance'");
+        }
+        b.append(" version='").append(Version.getInstance().getVersion()).append("'>\n");
+        SettingToXml toXml = new SettingToXml(b, nopass, defaults);
+        for (Entry<String, Setting<?>> e : settings) {
             toXml.setKey(e.getKey());
             e.getValue().visit(toXml);
         }
-        b.append("</preferences>\n");
+        b.append("</").append(rootElement).append(">\n");
         return b.toString();
     }

trunk/src/org/openstreetmap/josm/data/preferences/AbstractSetting.java

@@ -12 +12 @@
 public abstract class AbstractSetting<T> implements Setting<T> {
     protected final T value;
+    protected Long time;
+    protected boolean isNew;
     /**
      * Constructs a new {@code AbstractSetting} with the given value
@@ -18 +20 @@
     public AbstractSetting(T value) {
         this.value = value;
+        this.time = null;
+        this.isNew = false;
     }
 
@@ -23 +27 @@
     public T getValue() {
         return value;
+    }
+
+    @Override
+    public void setTime(Long time) {
+        this.time = time;
+    }
+
+    @Override
+    public Long getTime() {
+        return this.time;
+    }
+
+    @Override
+    public void setNew(boolean isNew) {
+        this.isNew = isNew;
+    }
+
+    @Override
+    public boolean isNew() {
+        return isNew;
     }
 

trunk/src/org/openstreetmap/josm/data/preferences/PreferencesReader.java

@@ -38 +38 @@
 public class PreferencesReader {
 
+    private static final String XSI_NS = "http://www.w3.org/2001/XMLSchema-instance";
+
     private final SortedMap<String, Setting<?>> settings = new TreeMap<>();
     private int version = 0;
     private XMLStreamReader parser;
+
+    private final boolean defaults;
+
+    /**
+     * Constructs a new {@code PreferencesReader}.
+     * @param defaults true when reading from the cache file for default preferences,
+     * false for the regular preferences config file
+     */
+    public PreferencesReader(boolean defaults) {
+        this.defaults = defaults;
+    }
 
     /**
@@ -111 +124 @@
         while (true) {
             if (event == XMLStreamConstants.START_ELEMENT) {
+                String topLevelElementName = defaults ? "preferences-defaults" : "preferences";
+                String localName = parser.getLocalName();
+                if (!topLevelElementName.equals(localName)) {
+                    throw new XMLStreamException(tr("Expected element ''{0}'', but got ''{1}''", topLevelElementName, localName), parser.getLocation());
+                }
                 try {
                     version = Integer.parseInt(parser.getAttributeValue(null, "version"));
@@ -138 +156 @@
                 switch(localName) {
                 case "tag":
-                    settings.put(parser.getAttributeValue(null, "key"), new StringSetting(parser.getAttributeValue(null, "value")));
+                    Setting setting;
+                    if (defaults && isNil()) {
+                        setting = new StringSetting(null);
+                    } else {
+                        String value = parser.getAttributeValue(null, "value");
+                        if (value == null) {
+                            throw new XMLStreamException(tr("value expected"), parser.getLocation());
+                        }
+                        setting = new StringSetting(value);
+                    }
+                    if (defaults) {
+                        setting.setTime(Math.round(Double.parseDouble(parser.getAttributeValue(null, "time"))));
+                    }
+                    settings.put(parser.getAttributeValue(null, "key"), setting);
                     jumpToEnd();
                     break;
                 case "list":
-                case "collection":
                 case "lists":
                 case "maps":
@@ -169 +199 @@
     private void parseToplevelList() throws XMLStreamException {
         String key = parser.getAttributeValue(null, "key");
+        Long time = null;
+        if (defaults) {
+            time = Math.round(Double.parseDouble(parser.getAttributeValue(null, "time")));
+        }
         String name = parser.getLocalName();
 
@@ -174 +208 @@
         List<List<String>> lists = null;
         List<Map<String, String>> maps = null;
-        while (true) {
-            int event = parser.next();
-            if (event == XMLStreamConstants.START_ELEMENT) {
-                String localName = parser.getLocalName();
-                switch(localName) {
-                case "entry":
-                    if (entries == null) {
-                        entries = new ArrayList<>();
+        if (defaults && isNil()) {
+            Setting setting;
+            switch (name) {
+                case "lists":
+                    setting = new ListListSetting(null);
+                    break;
+                case "maps":
+                    setting = new MapListSetting(null);
+                    break;
+                default:
+                    setting = new ListSetting(null);
+                    break;
+            }
+            setting.setTime(time);
+            settings.put(key, setting);
+            jumpToEnd();
+        } else {
+            while (true) {
+                int event = parser.next();
+                if (event == XMLStreamConstants.START_ELEMENT) {
+                    String localName = parser.getLocalName();
+                    switch(localName) {
+                    case "entry":
+                        if (entries == null) {
+                            entries = new ArrayList<>();
+                        }
+                        entries.add(parser.getAttributeValue(null, "value"));
+                        jumpToEnd();
+                        break;
+                    case "list":
+                        if (lists == null) {
+                            lists = new ArrayList<>();
+                        }
+                        lists.add(parseInnerList());
+                        break;
+                    case "map":
+                        if (maps == null) {
+                            maps = new ArrayList<>();
+                        }
+                        maps.add(parseMap());
+                        break;
+                    default:
+                        throwException("Unexpected element: "+localName);
                     }
-                    entries.add(parser.getAttributeValue(null, "value"));
-                    jumpToEnd();
-                    break;
-                case "list":
-                    if (lists == null) {
-                        lists = new ArrayList<>();
-                    }
-                    lists.add(parseInnerList());
-                    break;
-                case "map":
-                    if (maps == null) {
-                        maps = new ArrayList<>();
-                    }
-                    maps.add(parseMap());
-                    break;
-                default:
-                    throwException("Unexpected element: "+localName);
-                }
-            } else if (event == XMLStreamConstants.END_ELEMENT) {
-                break;
-            }
-        }
-        if (entries != null) {
-            settings.put(key, new ListSetting(Collections.unmodifiableList(entries)));
-        } else if (lists != null) {
-            settings.put(key, new ListListSetting(Collections.unmodifiableList(lists)));
-        } else if (maps != null) {
-            settings.put(key, new MapListSetting(Collections.unmodifiableList(maps)));
-        } else {
-            if ("lists".equals(name)) {
-                settings.put(key, new ListListSetting(Collections.<List<String>>emptyList()));
-            } else if ("maps".equals(name)) {
-                settings.put(key, new MapListSetting(Collections.<Map<String, String>>emptyList()));
+                } else if (event == XMLStreamConstants.END_ELEMENT) {
+                    break;
+                }
+            }
+            Setting setting;
+            if (entries != null) {
+                setting = new ListSetting(Collections.unmodifiableList(entries));
+            } else if (lists != null) {
+                setting = new ListListSetting(Collections.unmodifiableList(lists));
+            } else if (maps != null) {
+                setting = new MapListSetting(Collections.unmodifiableList(maps));
             } else {
-                settings.put(key, new ListSetting(Collections.<String>emptyList()));
-            }
+                switch (name) {
+                    case "lists":
+                        setting = new ListListSetting(Collections.<List<String>>emptyList());
+                        break;
+                    case "maps":
+                        setting = new MapListSetting(Collections.<Map<String, String>>emptyList());
+                        break;
+                    default:
+                        setting = new ListSetting(Collections.<String>emptyList());
+                        break;
+                }
+            }
+            if (defaults) {
+                setting.setTime(time);
+            }
+            settings.put(key, setting);
         }
     }
@@ -258 +319 @@
     }
 
+    /**
+     * Check if the current element is nil (meaning the value of the setting is null).
+     * @return true, if the current element is nil
+     * @see https://msdn.microsoft.com/en-us/library/2b314yt2(v=vs.85).aspx
+     */
+    private boolean isNil() {
+        String nil = parser.getAttributeValue(XSI_NS, "nil");
+        return "true".equals(nil) || "1".equals(nil);
+    }
+
+    /**
+     * Throw RuntimeException with line and column number.
+     *
+     * Only use this for errors that should not be possible after schema validation.
+     * @param msg the error message
+     */
     private void throwException(String msg) {
         throw new RuntimeException(msg + tr(" (at line {0}, column {1})",

trunk/src/org/openstreetmap/josm/data/preferences/Setting.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.data.preferences;
+
+import org.openstreetmap.josm.data.Preferences;
 
 /**
@@ -46 +48 @@
      */
     Setting<T> getNullInstance();
+
+    /**
+     * Set the time for this setting.
+     *
+     * For default preferences. They are saved in a cache file. Keeping the
+     * time allows to discard very old default settings.
+     * @param time the time in seconds since epoch
+     */
+    void setTime(Long time);
+
+    /**
+     * Get the time for this setting.
+     * @return the time for this setting
+     * @see #setTime(java.lang.Long)
+     */
+    Long getTime();
+
+    /**
+     * Mark setting as new.
+     *
+     * For default preferences. A setting is marked as new, if it has been seen
+     * in the current session.
+     * Methods like {@link Preferences#get(java.lang.String, java.lang.String)},
+     * can be called from different parts of the code with the same key. In this case,
+     * the supplied default value must match. However, this is only an error if the mismatching
+     * default value has been seen in the same session (and not loaded from cache).
+     * @param isNew true, if it is new
+     */
+    void setNew(boolean isNew);
+
+    /**
+     * Return if the setting has been marked as new.
+     * @return true, if the setting has been marked as new
+     * @see #setNew(boolean)
+     */
+    boolean isNew();
 }