Index: trunk/src/org/openstreetmap/josm/gui/io/SaveLayersDialog.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/io/SaveLayersDialog.java (revision 9555) +++ trunk/src/org/openstreetmap/josm/gui/io/SaveLayersDialog.java (revision 9556) @@ -9,4 +9,5 @@ import java.awt.Dimension; import java.awt.Graphics2D; +import java.awt.GraphicsEnvironment; import java.awt.GridBagConstraints; import java.awt.GridBagLayout; @@ -165,7 +166,13 @@ } - private static class LayerListWarningMessagePanel extends JPanel { - private JLabel lblMessage; - private JList lstLayers; + private static class LayerListWarningMessagePanel extends JPanel { + private final JLabel lblMessage = new JLabel(); + private final JList lstLayers = new JList<>(); + + LayerListWarningMessagePanel(String msg, List infos) { + build(); + lblMessage.setText(msg); + lstLayers.setListData(infos.toArray(new SaveLayerInfo[0])); + } protected void build() { @@ -177,7 +184,6 @@ gc.weightx = 1.0; gc.weighty = 0.0; - add(lblMessage = new JLabel(), gc); + add(lblMessage, gc); lblMessage.setHorizontalAlignment(JLabel.LEFT); - lstLayers = new JList<>(); lstLayers.setCellRenderer( new ListCellRenderer() { @@ -199,14 +205,16 @@ add(lstLayers, gc); } - - LayerListWarningMessagePanel(String msg, List infos) { - build(); - lblMessage.setText(msg); - lstLayers.setListData(infos.toArray(new SaveLayerInfo[0])); - } - } - - protected void warnLayersWithConflictsAndUploadRequest(List infos) { - String msg = trn("{0} layer has unresolved conflicts.
" + } + + private static void warn(String msg, List infos, String title) { + JPanel panel = new LayerListWarningMessagePanel(msg, infos); + // For unit test coverage in headless mode + if (!GraphicsEnvironment.isHeadless()) { + JOptionPane.showConfirmDialog(Main.parent, panel, title, JOptionPane.DEFAULT_OPTION, JOptionPane.WARNING_MESSAGE); + } + } + + protected static void warnLayersWithConflictsAndUploadRequest(List infos) { + warn(trn("{0} layer has unresolved conflicts.
" + "Either resolve them first or discard the modifications.
" + "Layer with conflicts:", @@ -215,16 +223,10 @@ + "Layers with conflicts:", infos.size(), - infos.size()); - JOptionPane.showConfirmDialog( - Main.parent, - new LayerListWarningMessagePanel(msg, infos), - tr("Unsaved data and conflicts"), - JOptionPane.DEFAULT_OPTION, - JOptionPane.WARNING_MESSAGE - ); - } - - protected void warnLayersWithoutFilesAndSaveRequest(List infos) { - String msg = trn("{0} layer needs saving but has no associated file.
" + infos.size()), + infos, tr("Unsaved data and conflicts")); + } + + protected static void warnLayersWithoutFilesAndSaveRequest(List infos) { + warn(trn("{0} layer needs saving but has no associated file.
" + "Either select a file for this layer or discard the changes.
" + "Layer without a file:", @@ -233,16 +235,10 @@ + "Layers without a file:", infos.size(), - infos.size()); - JOptionPane.showConfirmDialog( - Main.parent, - new LayerListWarningMessagePanel(msg, infos), - tr("Unsaved data and missing associated file"), - JOptionPane.DEFAULT_OPTION, - JOptionPane.WARNING_MESSAGE - ); - } - - protected void warnLayersWithIllegalFilesAndSaveRequest(List infos) { - String msg = trn("{0} layer needs saving but has an associated file
" + infos.size()), + infos, tr("Unsaved data and missing associated file")); + } + + protected static void warnLayersWithIllegalFilesAndSaveRequest(List infos) { + warn(trn("{0} layer needs saving but has an associated file
" + "which cannot be written.
" + "Either select another file for this layer or discard the changes.
" @@ -253,15 +249,9 @@ + "Layers with non-writable files:", infos.size(), - infos.size()); - JOptionPane.showConfirmDialog( - Main.parent, - new LayerListWarningMessagePanel(msg, infos), - tr("Unsaved data non-writable files"), - JOptionPane.DEFAULT_OPTION, - JOptionPane.WARNING_MESSAGE - ); - } - - protected boolean confirmSaveLayerInfosOK() { + infos.size()), + infos, tr("Unsaved data non-writable files")); + } + + static boolean confirmSaveLayerInfosOK(SaveLayersModel model) { List layerInfos = model.getLayersWithConflictsAndUploadRequest(); if (!layerInfos.isEmpty()) { @@ -388,10 +378,10 @@ final class SaveAndProceedAction extends AbstractAction implements PropertyChangeListener { - private static final int is = 24; // icon size + private static final int ICON_SIZE = 24; private static final String BASE_ICON = "BASE_ICON"; private final transient Image save = ImageProvider.get("save").getImage(); private final transient Image upld = ImageProvider.get("upload").getImage(); - private final transient Image saveDis = new BufferedImage(is, is, BufferedImage.TYPE_4BYTE_ABGR); - private final transient Image upldDis = new BufferedImage(is, is, BufferedImage.TYPE_4BYTE_ABGR); + private final transient Image saveDis = new BufferedImage(ICON_SIZE, ICON_SIZE, BufferedImage.TYPE_4BYTE_ABGR); + private final transient Image upldDis = new BufferedImage(ICON_SIZE, ICON_SIZE, BufferedImage.TYPE_4BYTE_ABGR); SaveAndProceedAction() { @@ -419,11 +409,12 @@ try { // Can fail if model is not yet setup properly Image base = ((ImageIcon) getValue(BASE_ICON)).getImage(); - BufferedImage newIco = new BufferedImage(is*3, is, BufferedImage.TYPE_4BYTE_ABGR); + BufferedImage newIco = new BufferedImage(ICON_SIZE*3, ICON_SIZE, BufferedImage.TYPE_4BYTE_ABGR); Graphics2D g = newIco.createGraphics(); - g.drawImage(model.getLayersToUpload().isEmpty() ? upldDis : upld, is*0, 0, is, is, null); - g.drawImage(model.getLayersToSave().isEmpty() ? saveDis : save, is*1, 0, is, is, null); - g.drawImage(base, is*2, 0, is, is, null); + g.drawImage(model.getLayersToUpload().isEmpty() ? upldDis : upld, ICON_SIZE*0, 0, ICON_SIZE, ICON_SIZE, null); + g.drawImage(model.getLayersToSave().isEmpty() ? saveDis : save, ICON_SIZE*1, 0, ICON_SIZE, ICON_SIZE, null); + g.drawImage(base, ICON_SIZE*2, 0, ICON_SIZE, ICON_SIZE, null); putValue(SMALL_ICON, new ImageIcon(newIco)); } catch (Exception e) { + Main.warn(e); putValue(SMALL_ICON, getValue(BASE_ICON)); } @@ -432,5 +423,5 @@ @Override public void actionPerformed(ActionEvent e) { - if (!confirmSaveLayerInfosOK()) + if (!confirmSaveLayerInfosOK(model)) return; launchSafeAndUploadTask(); Index: trunk/src/org/openstreetmap/josm/gui/io/SaveLayersModel.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/io/SaveLayersModel.java (revision 9555) +++ trunk/src/org/openstreetmap/josm/gui/io/SaveLayersModel.java (revision 9556) @@ -123,7 +123,9 @@ public List getLayersWithoutFilesAndSaveRequest() { List ret = new ArrayList<>(); - for (SaveLayerInfo info: layerInfo) { - if (info.isDoSaveToFile() && info.getFile() == null) { - ret.add(info); + if (layerInfo != null) { + for (SaveLayerInfo info: layerInfo) { + if (info.isDoSaveToFile() && info.getFile() == null) { + ret.add(info); + } } } @@ -133,7 +135,9 @@ public List getLayersWithIllegalFilesAndSaveRequest() { List ret = new ArrayList<>(); - for (SaveLayerInfo info: layerInfo) { - if (info.isDoSaveToFile() && info.getFile() != null && info.getFile().exists() && !info.getFile().canWrite()) { - ret.add(info); + if (layerInfo != null) { + for (SaveLayerInfo info: layerInfo) { + if (info.isDoSaveToFile() && info.getFile() != null && info.getFile().exists() && !info.getFile().canWrite()) { + ret.add(info); + } } } @@ -143,8 +147,10 @@ public List getLayersWithConflictsAndUploadRequest() { List ret = new ArrayList<>(); - for (SaveLayerInfo info: layerInfo) { - AbstractModifiableLayer l = info.getLayer(); - if (info.isDoUploadToServer() && l instanceof OsmDataLayer && !((OsmDataLayer) l).getConflicts().isEmpty()) { - ret.add(info); + if (layerInfo != null) { + for (SaveLayerInfo info: layerInfo) { + AbstractModifiableLayer l = info.getLayer(); + if (info.isDoUploadToServer() && l instanceof OsmDataLayer && !((OsmDataLayer) l).getConflicts().isEmpty()) { + ret.add(info); + } } } @@ -154,7 +160,9 @@ public List getLayersToUpload() { List ret = new ArrayList<>(); - for (SaveLayerInfo info: layerInfo) { - if (info.isDoUploadToServer()) { - ret.add(info); + if (layerInfo != null) { + for (SaveLayerInfo info: layerInfo) { + if (info.isDoUploadToServer()) { + ret.add(info); + } } } @@ -164,7 +172,9 @@ public List getLayersToSave() { List ret = new ArrayList<>(); - for (SaveLayerInfo info: layerInfo) { - if (info.isDoSaveToFile()) { - ret.add(info); + if (layerInfo != null) { + for (SaveLayerInfo info: layerInfo) { + if (info.isDoSaveToFile()) { + ret.add(info); + } } } Index: trunk/test/unit/org/openstreetmap/josm/gui/io/SaveLayersDialogTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/gui/io/SaveLayersDialogTest.java (revision 9556) +++ trunk/test/unit/org/openstreetmap/josm/gui/io/SaveLayersDialogTest.java (revision 9556) @@ -0,0 +1,55 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.gui.io; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.Collections; +import java.util.List; + +import org.junit.BeforeClass; +import org.junit.Test; +import org.openstreetmap.josm.JOSMFixture; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.gui.layer.OsmDataLayer; + +/** + * Unit tests of {@link SaveLayersDialog} class. + */ +public class SaveLayersDialogTest { + + /** + * Setup tests + */ + @BeforeClass + public static void setUpBeforeClass() { + JOSMFixture.createUnitTestFixture().init(); + } + + /** + * Test of {@link SaveLayersDialog#confirmSaveLayerInfosOK}. + */ + @Test + public void testConfirmSaveLayerInfosOK() { + final List list = Collections.singletonList(new SaveLayerInfo(new OsmDataLayer(new DataSet(), null, null))); + assertFalse(SaveLayersDialog.confirmSaveLayerInfosOK(new SaveLayersModel() { + @Override + public List getLayersWithConflictsAndUploadRequest() { + return list; + } + })); + assertFalse(SaveLayersDialog.confirmSaveLayerInfosOK(new SaveLayersModel() { + @Override + public List getLayersWithoutFilesAndSaveRequest() { + return list; + } + })); + assertFalse(SaveLayersDialog.confirmSaveLayerInfosOK(new SaveLayersModel() { + @Override + public List getLayersWithIllegalFilesAndSaveRequest() { + return list; + } + })); + assertTrue(SaveLayersDialog.confirmSaveLayerInfosOK(new SaveLayersModel())); + } +}