[vassalengine-svn-trunk] [vassalengine-svn] SF.net SVN: vassalengine:[8792] VASSAL-src/trunk
Brought to you by:
rodneykinney,
uckelman
From: <uck...@us...> - 2013-07-23 21:40:44
|
Revision: 8792 http://sourceforge.net/p/vassalengine/svn/8792 Author: uckelman Date: 2013-07-23 21:40:37 +0000 (Tue, 23 Jul 2013) Log Message: ----------- Preferences are now stored in the prefs directory, one file per module, instead of in the Preferences ZIP archive. This should eliminate problems with the Module Manager and Player stepping on each other's toes when reading/writing prefs. This fixes: * Bug 10266: ConcurrentModificationException at Prefs.save() * Bug 3294: IOException writing preferences at startup on Mac OS * Bug 2727: NPE in ReadOnlyPrefs ctor * Bug 2587: Manager and Player preferences get out of step Modified Paths: -------------- VASSAL-src/trunk/README VASSAL-src/trunk/src/VASSAL/Info.java VASSAL-src/trunk/src/VASSAL/launch/ModuleManager.java VASSAL-src/trunk/src/VASSAL/launch/ModuleManagerWindow.java VASSAL-src/trunk/src/VASSAL/preferences/Prefs.java VASSAL-src/trunk/src/VASSAL/preferences/PrefsEditor.java VASSAL-src/trunk/src/VASSAL/preferences/ReadOnlyPrefs.java Removed Paths: ------------- VASSAL-src/trunk/src/VASSAL/preferences/GlobalPrefs.java Modified: VASSAL-src/trunk/README =================================================================== --- VASSAL-src/trunk/README 2013-07-23 21:14:48 UTC (rev 8791) +++ VASSAL-src/trunk/README 2013-07-23 21:40:37 UTC (rev 8792) @@ -183,6 +183,10 @@ differently subsampled are incorrectly color-corrected by fix for Bug 9882 * 10298: Exception: TurnTracker.level_error in TurnComponent.getTurnLevel() * 10279: Report State failed to perform inner commands before its own command +* 10266: ConcurrentModificationException at Prefs.save() +* 3294: IOException writing preferences at startup on Mac OS +* 2727: NPE in ReadOnlyPrefs ctor +* 2587: Manager and Player preferences get out of step * Reverted fix for 2714: NPE in StackMetrics.merge() Modified: VASSAL-src/trunk/src/VASSAL/Info.java =================================================================== --- VASSAL-src/trunk/src/VASSAL/Info.java 2013-07-23 21:14:48 UTC (rev 8791) +++ VASSAL-src/trunk/src/VASSAL/Info.java 2013-07-23 21:40:37 UTC (rev 8792) @@ -35,7 +35,7 @@ * Class for storing release-related information */ public final class Info { - private static final String VERSION = "3.2.6"; //$NON-NLS-1$ + private static final String VERSION = "3.2.8-svn8790"; //$NON-NLS-1$ // Do not allow editing of modules with this revision or later private static final String EXPIRY_VERSION = "3.3"; //$NON-NLS-1$ @@ -233,6 +233,10 @@ return new File(getHomeDir(), "tmp"); } + public static File getPrefsDir() { + return new File(getConfDir(), "prefs"); + } + // FIXME: this is a misleading name for this function public static File getHomeDir() { // FIXME: creation of VASSAL's home should be moved someplace else, possibly Modified: VASSAL-src/trunk/src/VASSAL/launch/ModuleManager.java =================================================================== --- VASSAL-src/trunk/src/VASSAL/launch/ModuleManager.java 2013-07-23 21:14:48 UTC (rev 8791) +++ VASSAL-src/trunk/src/VASSAL/launch/ModuleManager.java 2013-07-23 21:40:37 UTC (rev 8792) @@ -27,6 +27,7 @@ import java.io.InputStream; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; +import java.io.OutputStream; import java.io.PrintStream; import java.io.RandomAccessFile; import java.lang.reflect.InvocationTargetException; @@ -58,6 +59,7 @@ import VASSAL.tools.ErrorDialog; import VASSAL.tools.ThrowableUtils; import VASSAL.tools.io.IOUtils; +import VASSAL.tools.io.ZipArchive; import VASSAL.tools.logging.LoggedOutputStream; import VASSAL.tools.menu.MacOSXMenuManager; import VASSAL.tools.menu.MenuBarProxy; @@ -282,14 +284,59 @@ start.initSystemProperties(); - // check whether we need to migrate pre-3.2 preferences - final File oldprefs = - new File(System.getProperty("user.home"), "VASSAL/Preferences"); - if (oldprefs.exists()) { - final File newprefs = new File(Info.getHomeDir(), "Preferences"); - if (!newprefs.exists()) { - FileUtils.copyFile(oldprefs, newprefs); + // try to migrate old preferences if there are no current ones + final File pdir = Info.getPrefsDir(); + if (!pdir.exists()) { + // Check the 3.2.0 through 3.2.7 location + File pzip = new File(Info.getHomeDir(), "Preferences"); + if (!pzip.exists()) { + // Check the pre-3.2 location. + pzip = new File(System.getProperty("user.home"), "VASSAL/Preferences"); } + + if (pzip.exists()) { + FileUtils.forceMkdir(pdir); + + final byte[] buf = new byte[4096]; + + try { + final ZipArchive za = new ZipArchive(pzip); + try { + for (String f : za.getFiles()) { + final File ofile = new File( + pdir, "VASSAL".equals(f) ? "V_Global" : Prefs.sanitize(f) + ); + + InputStream in = null; + try { + in = za.getInputStream(f); + + OutputStream out = null; + try { + out = new FileOutputStream(ofile); + IOUtils.copy(in, out, buf); + out.close(); + } + finally { + IOUtils.closeQuietly(out); + } + + in.close(); + } + finally { + IOUtils.closeQuietly(in); + } + } + za.close(); + } + finally { + IOUtils.closeQuietly(za); + } + } + catch (IOException e) { + logger.error("Failed to convert legacy preferences file.", e); + } + } } if (SystemUtils.IS_OS_MAC_OSX) new MacOSXMenuManager(); @@ -453,8 +500,7 @@ final ModuleManagerWindow window = ModuleManagerWindow.getInstance(); window.setVisible(true); - final File prefsFile = new File(Info.getHomeDir(), "Preferences"); - final boolean isFirstTime = !prefsFile.exists(); + final boolean isFirstTime = !Info.getPrefsDir().exists(); if (isFirstTime) new FirstTimeDialog(window).setVisible(true); } Modified: VASSAL-src/trunk/src/VASSAL/launch/ModuleManagerWindow.java =================================================================== --- VASSAL-src/trunk/src/VASSAL/launch/ModuleManagerWindow.java 2013-07-23 21:14:48 UTC (rev 8791) +++ VASSAL-src/trunk/src/VASSAL/launch/ModuleManagerWindow.java 2013-07-23 21:40:37 UTC (rev 8792) @@ -115,6 +115,7 @@ import VASSAL.tools.WriteErrorDialog; import VASSAL.tools.filechooser.FileChooser; import VASSAL.tools.filechooser.ModuleExtensionFileFilter; +import VASSAL.tools.io.IOUtils; import VASSAL.tools.logging.LogPane; import VASSAL.tools.menu.CheckBoxMenuItemProxy; import VASSAL.tools.menu.MenuBarProxy; @@ -188,6 +189,17 @@ public void actionPerformed(ActionEvent e) { if (!AbstractLaunchAction.shutDown()) return; + final Prefs gp = Prefs.getGlobalPrefs(); + try { + gp.close(); + } + catch (IOException ex) { + WriteErrorDialog.error(ex, gp.getFile()); + } + finally { + IOUtils.closeQuietly(gp); + } + // Bug 10179 - Global prefs are now written out each time a preference is changed // final Prefs gl = Prefs.getGlobalPrefs(); // try { Deleted: VASSAL-src/trunk/src/VASSAL/preferences/GlobalPrefs.java =================================================================== --- VASSAL-src/trunk/src/VASSAL/preferences/GlobalPrefs.java 2013-07-23 21:14:48 UTC (rev 8791) +++ VASSAL-src/trunk/src/VASSAL/preferences/GlobalPrefs.java 2013-07-23 21:40:37 UTC (rev 8792) @@ -1,60 +0,0 @@ -/* - * $Id: GlobakPrefs.java 8428 2012-11-14 19:29:03Z uckelman $ - * - * Copyright (c) 2000-2013 by Rodney Kinney, Brent Easton - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Library General Public - * License (LGPL) as published by the Free Software Foundation. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Library General Public License for more details. - * - * You should have received a copy of the GNU Library General Public - * License along with this library; if not, copies are available - * at http://www.opensource.org. - */ - -/** - * Bug 10179 - * Global preferences are now written to disk each time a value is changed. - * Each time a value is read, check first if there is an updated copy of prefs on disk - * and read them in first. - */ -package VASSAL.preferences; - -import java.beans.PropertyChangeEvent; -import java.io.IOException; - -import VASSAL.tools.WriteErrorDialog; - -public class GlobalPrefs extends Prefs { - - protected long dtm = 0; - - public GlobalPrefs(PrefsEditor editor, String name) { - super(editor, name); - } - - protected void handleValueChange(PropertyChangeEvent evt) { - super.handleValueChange(evt); - try { - write(); - } - catch (IOException ex) { - WriteErrorDialog.error(ex, getFile().getPath()); - } - } - - public Object getValue(String key) { - final long currentDtm = getFile().lastModified(); - if (currentDtm != dtm) { - dtm = currentDtm; - read(); - init(name); - } - return super.getValue(key); - } -} \ No newline at end of file Modified: VASSAL-src/trunk/src/VASSAL/preferences/Prefs.java =================================================================== --- VASSAL-src/trunk/src/VASSAL/preferences/Prefs.java 2013-07-23 21:14:48 UTC (rev 8791) +++ VASSAL-src/trunk/src/VASSAL/preferences/Prefs.java 2013-07-23 21:40:37 UTC (rev 8792) @@ -18,20 +18,22 @@ */ package VASSAL.preferences; -import java.beans.PropertyChangeEvent; -import java.beans.PropertyChangeListener; import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; import java.io.Closeable; import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.InputStream; import java.io.IOException; import java.io.OutputStream; import java.util.Enumeration; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; import java.util.Properties; -import java.util.Set; +import org.apache.commons.io.FileUtils; import org.apache.commons.lang.SystemUtils; import VASSAL.Info; @@ -41,9 +43,7 @@ import VASSAL.configure.DirectoryConfigurer; import VASSAL.i18n.Resources; import VASSAL.tools.ReadErrorDialog; -import VASSAL.tools.io.FileArchive; import VASSAL.tools.io.IOUtils; -import VASSAL.tools.io.ZipArchive; /** * A set of preferences. Each set of preferences is identified by a name, and different sets may share a common editor, @@ -54,36 +54,44 @@ public static final String MODULES_DIR_KEY = "modulesDir"; // $NON_NLS-1$ public static final String DISABLE_D3D = "disableD3d"; public static final String DISABLE_QUARTZ = "disableD3d"; + private static Prefs globalPrefs; + private Map<String, Configurer> options = new HashMap<String, Configurer>(); private Properties storedValues = new Properties(); private PrefsEditor editor; - protected String name; - private Set<String> changed = new HashSet<String>(); - private PropertyChangeListener changeListener; + private File file; public Prefs(PrefsEditor editor, String name) { + this(editor, new File(Info.getPrefsDir(), sanitize(name))); + } + + protected Prefs(PrefsEditor editor, File file) { this.editor = editor; - this.name = name; - this.changeListener = new PropertyChangeListener() { - public void propertyChange(PropertyChangeEvent evt) { - handleValueChange(evt); + this.file = file; + + read(); + + // FIXME: Use stringPropertyNames() in 1.6+ + // for (String key : storedValues.stringPropertyNames()) { + for (Enumeration<?> e = storedValues.keys(); e.hasMoreElements();) { + final String key = (String) e.nextElement(); + final String value = storedValues.getProperty(key); + final Configurer c = options.get(key); + if (c != null) { + c.setValue(value); } - }; + } + editor.addPrefs(this); - init(name); } - protected void handleValueChange(PropertyChangeEvent evt) { - changed.add(evt.getPropertyName()); - } - public PrefsEditor getEditor() { return editor; } public File getFile() { - return new File(editor.getFileArchive().getName()); + return file; } public void addOption(Configurer o) { @@ -116,7 +124,6 @@ editor.addOption(category, o, prompt); } } - o.addPropertyChangeListener(changeListener); } public void setValue(String option, Object value) { @@ -148,53 +155,50 @@ return storedValues.getProperty(key); } - public void init(String moduleName) { - name = moduleName; - read(); - - try { - editor.close(); + public static String sanitize(String str) { + /* + Java gives us no way of checking whether a string is a valid + filename on the filesystem we're using. Filenames matching + [0-9A-Za-z_]+ are safe pretty much everywhere. Any code point + in [0-9A-Za-z] is passed through; every other code point c is + escaped as "_hex(c)_". This mapping is a surjection and will + produce filenames safe on every sane filesystem, so long as the + input strings are not too long. + */ + final StringBuilder sb = new StringBuilder(); + for (int i = 0; i < str.length(); ++i) { + final int cp = str.codePointAt(i); + if (('0' <= cp && cp <= '9') || + ('A' <= cp && cp <= 'Z') || + ('a' <= cp && cp <= 'z')) { + sb.append((char) cp); + } + else { + sb.append('_') + .append(Integer.toHexString(cp).toUpperCase()) + .append('_'); + } } - catch (IOException e) { -// FIXME - } - // FIXME: Use stringPropertyNames() in 1.6+ - // for (String key : storedValues.stringPropertyNames()) { - for (Enumeration<?> e = storedValues.keys(); e.hasMoreElements();) { - final String key = (String) e.nextElement(); - final String value = storedValues.getProperty(key); - final Configurer c = options.get(key); - if (c != null) { - c.setValue(value); - } - } + return sb.toString(); } protected void read() { - FileArchive fa = null; + InputStream in = null; try { - fa = editor.getFileArchive(); - if (fa.contains(name)) { - BufferedInputStream in = null; - try { - in = new BufferedInputStream(fa.getInputStream(name)); - storedValues.clear(); - storedValues.load(in); - in.close(); - } - finally { - IOUtils.closeQuietly(in); - } - } - - fa.close(); + in = new BufferedInputStream(new FileInputStream(file)); + storedValues.clear(); + storedValues.load(in); + in.close(); } + catch (FileNotFoundException e) { + // First time for this module, not an error. + } catch (IOException e) { - ReadErrorDialog.errorNoI18N(e, fa.getName()); + ReadErrorDialog.errorNoI18N(e, file); } finally { - IOUtils.closeQuietly(fa); + IOUtils.closeQuietly(in); } } @@ -202,39 +206,40 @@ * Store this set of preferences in the editor, but don't yet save to disk */ public void save() throws IOException { + // collect the key-value pairs + storedValues.clear(); + for (Configurer c : options.values()) { - if (changed.contains(c.getKey())) { - final String val = c.getValueString(); - if (val != null) { - storedValues.put(c.getKey(), val); - } - else { - storedValues.remove(c.getKey()); - } + final String val = c.getValueString(); + if (val != null) { + storedValues.put(c.getKey(), val); } } - FileArchive fa = editor.getFileArchive(); + // ensure that the prefs dir exists + if (!Info.getPrefsDir().exists()) { + FileUtils.forceMkdir(Info.getPrefsDir()); + } + + // write the key-value pairs OutputStream out = null; try { - out = fa.getOutputStream(name); + out = new BufferedOutputStream(new FileOutputStream(file)); storedValues.store(out, null); out.close(); } finally { IOUtils.closeQuietly(out); } - - changed.clear(); } /** Save these preferences and write to disk. */ public void write() throws IOException { - editor.write(); + save(); } public void close() throws IOException { - editor.close(); + save(); if (this == globalPrefs) { globalPrefs = null; } @@ -247,17 +252,10 @@ */ public static Prefs getGlobalPrefs() { if (globalPrefs == null) { - final File prefsFile = new File(Info.getHomeDir(), "Preferences"); + final PrefsEditor ed = new PrefsEditor(); + // The underscore prevents collisions with module prefs + globalPrefs = new Prefs(ed, new File(Info.getPrefsDir(), "V_Global")); - try { - globalPrefs = new GlobalPrefs( - new PrefsEditor(new ZipArchive(prefsFile)), "VASSAL" - ); - } - catch (IOException e) { - ReadErrorDialog.error(e, prefsFile); - } - final DirectoryConfigurer c = new DirectoryConfigurer(MODULES_DIR_KEY, null); c.setValue(new File(System.getProperty("user.home"))); Modified: VASSAL-src/trunk/src/VASSAL/preferences/PrefsEditor.java =================================================================== --- VASSAL-src/trunk/src/VASSAL/preferences/PrefsEditor.java 2013-07-23 21:14:48 UTC (rev 8791) +++ VASSAL-src/trunk/src/VASSAL/preferences/PrefsEditor.java 2013-07-23 21:40:37 UTC (rev 8792) @@ -27,6 +27,7 @@ import java.awt.event.ComponentEvent; import java.awt.event.WindowAdapter; import java.awt.event.WindowEvent; +import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; @@ -50,7 +51,6 @@ import VASSAL.tools.ArchiveWriter; import VASSAL.tools.SplashScreen; import VASSAL.tools.WriteErrorDialog; -import VASSAL.tools.io.FileArchive; import VASSAL.tools.io.ZipArchive; public class PrefsEditor { @@ -58,26 +58,15 @@ private List<Configurer> options = new ArrayList<Configurer>(); private List<Configurer> extras = new ArrayList<Configurer>(); private boolean iterating = false; - private Map<Configurer, Object> savedValues; - private List<Prefs> prefs; - private JTabbedPane optionsTab; + private Map<Configurer,Object> savedValues = new HashMap<Configurer,Object>(); + private List<Prefs> prefs = new ArrayList<Prefs>(); + private JTabbedPane optionsTab = new JTabbedPane(); private JDialog setupDialog; - private FileArchive archive; + private File pfile; private Action editAction; - public PrefsEditor(FileArchive archive) { - savedValues = new HashMap<Configurer, Object>(); - this.archive = archive; - prefs = new ArrayList<Prefs>(); - optionsTab = new JTabbedPane(); - } + public PrefsEditor() {} - /** @deprecated Use {@link PrefsEditor(FileArchive)} instead. */ - @Deprecated - public PrefsEditor(ArchiveWriter archive) throws IOException { - this(new ZipArchive(archive.getName())); - } - public void initDialog(Frame parent) { if (dialog == null) { dialog = new JDialog(parent, true); @@ -148,7 +137,10 @@ setupDialog.add(p); setupDialog.pack(); Dimension d = Toolkit.getDefaultToolkit().getScreenSize(); - setupDialog.setLocation(d.width / 2 - setupDialog.getSize().width / 2, d.height / 2 - setupDialog.getSize().height / 2); + setupDialog.setLocation( + d.width / 2 - setupDialog.getSize().width / 2, + d.height / 2 - setupDialog.getSize().height / 2 + ); setupDialog.setVisible(true); setupDialog.removeAll(); } @@ -195,10 +187,6 @@ } } - public FileArchive getFileArchive() { - return archive; - } - protected synchronized void cancel() { for (Configurer c : options) { c.setValue(savedValues.get(c)); @@ -220,12 +208,7 @@ options.addAll(extras); extras.clear(); - try { - write(); - } - catch (IOException e) { - WriteErrorDialog.error(e, archive.getName()); - } + write(); dialog.setVisible(false); } @@ -251,12 +234,18 @@ return editAction; } - public void write() throws IOException { - for (Prefs p : prefs) p.save(); - archive.flush(); + public void write() { + for (Prefs p : prefs) { + try { + p.save(); + } + catch (IOException e) { + WriteErrorDialog.error(e, p.getFile()); + } + } } - public void close() throws IOException { - archive.close(); + public void close() { + write(); } } Modified: VASSAL-src/trunk/src/VASSAL/preferences/ReadOnlyPrefs.java =================================================================== --- VASSAL-src/trunk/src/VASSAL/preferences/ReadOnlyPrefs.java 2013-07-23 21:14:48 UTC (rev 8791) +++ VASSAL-src/trunk/src/VASSAL/preferences/ReadOnlyPrefs.java 2013-07-23 21:40:37 UTC (rev 8792) @@ -21,56 +21,46 @@ import java.io.BufferedInputStream; import java.io.File; +import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.util.Properties; -import java.util.zip.ZipEntry; -import java.util.zip.ZipFile; import VASSAL.Info; import VASSAL.tools.ReadErrorDialog; import VASSAL.tools.io.IOUtils; /** - * A simple prefernces class which permits reading stored values. + * A simple preferences class which permits reading stored values. * * @author Joel Uckelman * @since 3.1.0 */ public class ReadOnlyPrefs { protected Properties storedValues = new Properties(); - protected String name; /** * @param name the module name of the preferences to read */ public ReadOnlyPrefs(String name) { - this.name = name; + this(new File(Info.getPrefsDir(), Prefs.sanitize(name))); + } - final File zipfile = new File(Info.getHomeDir(), "Preferences"); - - ZipFile zip = null; + protected ReadOnlyPrefs(File file) { BufferedInputStream in = null; try { - zip = new ZipFile(zipfile); - final ZipEntry entry = zip.getEntry(name); - if (entry == null) - throw new FileNotFoundException(name + " does not exist"); - - in = new BufferedInputStream(zip.getInputStream(entry)); + in = new BufferedInputStream(new FileInputStream(file)); storedValues.load(in); in.close(); - zip.close(); } catch (FileNotFoundException e) { // First time for this module, not an error. } catch (IOException e) { - ReadErrorDialog.error(e, zipfile.getPath() + "/!" + name); + ReadErrorDialog.error(e, file); } finally { IOUtils.closeQuietly(in); - IOUtils.closeQuietly(zip); } } @@ -91,6 +81,6 @@ * @return a global preferences object */ public static ReadOnlyPrefs getGlobalPrefs() { - return new ReadOnlyPrefs("VASSAL"); + return new ReadOnlyPrefs(new File(Info.getPrefsDir(), "V_Global")); } } This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site. ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk _______________________________________________ vassalengine-svn mailing list vas...@li... https://lists.sourceforge.net/lists/listinfo/vassalengine-svn |