Index: src/org/openstreetmap/josm/actions/UploadAction.java =================================================================== --- src/org/openstreetmap/josm/actions/UploadAction.java (revision 18270) +++ src/org/openstreetmap/josm/actions/UploadAction.java (working copy) @@ -35,6 +35,7 @@ import org.openstreetmap.josm.io.UploadStrategySpecification; import org.openstreetmap.josm.spi.preferences.Config; import org.openstreetmap.josm.tools.ImageProvider; +import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Shortcut; import org.openstreetmap.josm.tools.Utils; @@ -239,8 +240,8 @@ ChangesetUpdater.check(); final UploadDialog dialog = UploadDialog.getUploadDialog(); + dialog.setUploadedPrimitives(apiData); dialog.initLifeCycle(layer.getDataSet()); - dialog.setUploadedPrimitives(apiData); dialog.setVisible(true); dialog.rememberUserInput(); if (dialog.isCanceled()) { @@ -266,6 +267,9 @@ } UploadStrategySpecification uploadStrategySpecification = dialog.getUploadStrategySpecification(); + Logging.info("Starting upload with tags {0}", changesetTags); + Logging.info(uploadStrategySpecification.toString()); + Logging.info(cs.toString()); dialog.clean(); if (Config.getPref().getBoolean(IS_ASYNC_UPLOAD_ENABLED, true)) { Index: src/org/openstreetmap/josm/data/osm/Changeset.java =================================================================== --- src/org/openstreetmap/josm/data/osm/Changeset.java (revision 18270) +++ src/org/openstreetmap/josm/data/osm/Changeset.java (working copy) @@ -317,7 +317,7 @@ .ifPresent(value -> { throw new IllegalArgumentException("Changeset tag value is too long: "+value); }); - this.tags = keys; + this.tags = new HashMap<>(keys); } /** Index: src/org/openstreetmap/josm/data/osm/ChangesetCache.java =================================================================== --- src/org/openstreetmap/josm/data/osm/ChangesetCache.java (revision 18270) +++ src/org/openstreetmap/josm/data/osm/ChangesetCache.java (working copy) @@ -12,9 +12,13 @@ import java.util.stream.Collectors; import org.openstreetmap.josm.data.UserIdentityManager; +import org.openstreetmap.josm.io.ChangesetQuery; +import org.openstreetmap.josm.io.OsmServerChangesetReader; +import org.openstreetmap.josm.io.OsmTransferException; import org.openstreetmap.josm.spi.preferences.Config; import org.openstreetmap.josm.spi.preferences.PreferenceChangeEvent; import org.openstreetmap.josm.spi.preferences.PreferenceChangedListener; +import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.SubclassFilteredCollection; import org.openstreetmap.josm.tools.Utils; @@ -254,6 +258,36 @@ } } + /** + * Refreshes the changesets from the server. + *

+ * The server automatically closes changesets after a timeout. We don't get notified of this + * fact when it happens. This method requests a fresh list from the server and updates the + * local list. Calling this method reduces (but does not eliminate) the probability of + * attempting an upload to an already closed changeset. + * + * @throws OsmTransferException on server error + */ + public void refreshChangesetsFromServer() throws OsmTransferException { + OsmServerChangesetReader reader; + synchronized (this) { + reader = new OsmServerChangesetReader(); + } + List server = reader.queryChangesets(ChangesetQuery.forCurrentUser().beingOpen(true), null); + Logging.info("{0} Open changesets on server", server.size()); + + DefaultChangesetCacheEvent e = new DefaultChangesetCacheEvent(this); + // flag timed out changesets + for (Changeset cs : getOpenChangesetsForCurrentUser()) { + if (!server.contains(cs)) + remove(cs.getId(), e); + } + for (Changeset cs: server) { + update(cs, e); + } + fireChangesetCacheEvent(e); + } + /* ------------------------------------------------------------------------- */ /* interface PreferenceChangedListener */ /* ------------------------------------------------------------------------- */ Index: src/org/openstreetmap/josm/gui/io/BasicUploadSettingsPanel.java =================================================================== --- src/org/openstreetmap/josm/gui/io/BasicUploadSettingsPanel.java (revision 18270) +++ src/org/openstreetmap/josm/gui/io/BasicUploadSettingsPanel.java (working copy) @@ -102,6 +102,21 @@ build(); } + protected void build() { + setLayout(new GridBagLayout()); + setBorder(BorderFactory.createEmptyBorder(3, 3, 3, 3)); + GBC gbc = GBC.eop().fill(GBC.HORIZONTAL); + add(buildUploadCommentPanel(), gbc); + add(buildUploadSourcePanel(), gbc); + add(pnlUploadParameterSummary, gbc); + if (Config.getPref().getBoolean("upload.show.review.request", true)) { + add(cbRequestReview, gbc); + cbRequestReview.addItemListener(this); + } + add(areaValidatorFeedback, gbc); + add(new JPanel(), GBC.std().fill(GBC.BOTH)); + } + protected JPanel buildUploadCommentPanel() { JPanel pnl = new JPanel(new GridBagLayout()); pnl.setBorder(BorderFactory.createTitledBorder(tr("Provide a brief comment for the changes you are uploading:"))); @@ -113,8 +128,9 @@ editor.addKeyListener(this); editor.addFocusListener(this); editor.addActionListener(this); - pnl.add(hcbUploadComment, GBC.eol().fill(GBC.HORIZONTAL)); - pnl.add(uploadCommentFeedback, GBC.eol().insets(0, 3, 0, 0).fill(GBC.HORIZONTAL)); + GBC gbc = GBC.eol().insets(3).fill(GBC.HORIZONTAL); + pnl.add(hcbUploadComment, gbc); + pnl.add(uploadCommentFeedback, gbc); return pnl; } @@ -141,9 +157,6 @@ obtainSource.add(obtainSourceAutomatically, GBC.std().anchor(GBC.WEST)); obtainSource.add(obtainSourceOnce, GBC.std().anchor(GBC.WEST)); obtainSource.add(new JLabel(), GBC.eol().fill(GBC.HORIZONTAL)); - if (Config.getPref().getBoolean("upload.show.automatic.source", true)) { - pnl.add(obtainSource, GBC.eol().insets(0, 0, 10, 3).fill(GBC.HORIZONTAL)); - } hcbUploadSource.setToolTipText(tr("Enter a source")); hcbUploadSource.getEditorComponent().setMaxTextLength(Changeset.MAX_CHANGESET_TAG_LENGTH); @@ -152,9 +165,12 @@ editor.addKeyListener(this); editor.addFocusListener(this); editor.addActionListener(this); - pnl.add(hcbUploadSource, GBC.eol().fill(GBC.HORIZONTAL)); - pnl.add(hcbUploadSourceFeedback, GBC.eol().insets(0, 3, 0, 0).fill(GBC.HORIZONTAL)); - + GBC gbc = GBC.eol().insets(3).fill(GBC.HORIZONTAL); + if (Config.getPref().getBoolean("upload.show.automatic.source", true)) { + pnl.add(obtainSource, gbc); + } + pnl.add(hcbUploadSource, gbc); + pnl.add(hcbUploadSourceFeedback, gbc); return pnl; } @@ -161,7 +177,8 @@ /** * Initializes this life cycle of the panel. * - * Adds any changeset tags to the map. + * Adds the comment and source tags from history, and/or obtains the source from the layer if + * the user said so. * * @param map Map where tags are added to. * @since 18173 @@ -178,6 +195,9 @@ hcbUploadComment.discardAllUndoableEdits(); hcbUploadSource.getModel().prefs().load(SOURCE_HISTORY_KEY, getDefaultSources()); hcbUploadSource.discardAllUndoableEdits(); + hcbUploadComment.getEditorComponent().requestFocusInWindow(); + uploadCommentValidator.validate(); + uploadSourceValidator.validate(); } /** @@ -234,21 +254,6 @@ return Arrays.asList(areaValidator, uploadCommentValidator, uploadSourceValidator); } - protected void build() { - setLayout(new GridBagLayout()); - setBorder(BorderFactory.createEmptyBorder(3, 3, 3, 3)); - GBC gbc = GBC.eol().insets(0, 0, 0, 20).fill(GBC.HORIZONTAL); - add(buildUploadCommentPanel(), gbc); - add(buildUploadSourcePanel(), gbc); - add(pnlUploadParameterSummary, gbc); - if (Config.getPref().getBoolean("upload.show.review.request", true)) { - add(cbRequestReview, gbc); - cbRequestReview.addItemListener(this); - } - add(areaValidatorFeedback, gbc); - add(new JPanel(), GBC.std().fill(GBC.BOTH)); - } - /** * Remembers the user input in the preference settings */ @@ -268,15 +273,6 @@ } /** - * Initializes the panel for user input - */ - public void startUserInput() { - hcbUploadComment.getEditorComponent().requestFocusInWindow(); - uploadCommentValidator.validate(); - uploadSourceValidator.validate(); - } - - /** * Initializes editing of upload comment. */ public void initEditingOfUploadComment() { Index: src/org/openstreetmap/josm/gui/io/ChangesetCellRenderer.java =================================================================== --- src/org/openstreetmap/josm/gui/io/ChangesetCellRenderer.java (revision 18270) +++ src/org/openstreetmap/josm/gui/io/ChangesetCellRenderer.java (working copy) @@ -62,19 +62,24 @@ } if (cs != null) { setIcon(icon); - StringBuilder sb = new StringBuilder(); - String comment = cs.getComment(); - if (!comment.isEmpty()) { - sb.append(cs.getId()).append(" - ").append(comment); - } else if (cs.get("name") != null) { - sb.append(cs.getId()).append(" - ").append(cs.get("name")); + if (cs.getId() == 0) { + setText("New changeset"); } else { - sb.append(tr("Changeset {0}", cs.getId())); + StringBuilder sb = new StringBuilder(); + String comment = cs.getComment(); + if (!comment.isEmpty()) { + sb.append(cs.getId()).append(" - ").append(comment); + } else if (cs.get("name") != null) { + sb.append(cs.getId()).append(" - ").append(cs.get("name")); + } else { + sb.append(tr("Changeset {0}", cs.getId())); + } + setText(sb.toString()); } - setText(sb.toString()); setToolTipText(buildToolTipText(cs)); } else { - setText(tr("No open changesets")); + setIcon(null); + setText(""); } return this; } Index: src/org/openstreetmap/josm/gui/io/ChangesetManagementPanel.java =================================================================== --- src/org/openstreetmap/josm/gui/io/ChangesetManagementPanel.java (revision 18270) +++ src/org/openstreetmap/josm/gui/io/ChangesetManagementPanel.java (working copy) @@ -11,22 +11,24 @@ import java.awt.event.ItemEvent; import java.awt.event.ItemListener; import java.util.Collections; +import java.util.Optional; import javax.swing.AbstractAction; import javax.swing.BorderFactory; -import javax.swing.ButtonGroup; import javax.swing.JButton; import javax.swing.JCheckBox; import javax.swing.JPanel; -import javax.swing.JRadioButton; -import javax.swing.event.ListDataEvent; -import javax.swing.event.ListDataListener; +import javax.swing.SwingUtilities; import org.openstreetmap.josm.data.osm.Changeset; import org.openstreetmap.josm.data.osm.ChangesetCache; +import org.openstreetmap.josm.data.osm.ChangesetCacheEvent; +import org.openstreetmap.josm.data.osm.ChangesetCacheListener; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.gui.widgets.JMultilineLabel; import org.openstreetmap.josm.gui.widgets.JosmComboBox; +import org.openstreetmap.josm.gui.widgets.JosmComboBoxModel; +import org.openstreetmap.josm.io.OsmTransferException; import org.openstreetmap.josm.spi.preferences.Config; import org.openstreetmap.josm.tools.ImageProvider; @@ -44,157 +46,129 @@ * whether the changeset should be closed after the next upload * */ -public class ChangesetManagementPanel extends JPanel implements ListDataListener { +public class ChangesetManagementPanel extends JPanel implements ItemListener, ChangesetCacheListener { static final String SELECTED_CHANGESET_PROP = ChangesetManagementPanel.class.getName() + ".selectedChangeset"; static final String CLOSE_CHANGESET_AFTER_UPLOAD = ChangesetManagementPanel.class.getName() + ".closeChangesetAfterUpload"; - private JRadioButton rbUseNew; - private JRadioButton rbExisting; private JosmComboBox cbOpenChangesets; + private JosmComboBoxModel model; private JCheckBox cbCloseAfterUpload; - private OpenChangesetComboBoxModel model; + private JButton btnClose; - /** the changeset comment model */ - private final transient UploadDialogModel uploadDialogModel; - /** * Constructs a new {@code ChangesetManagementPanel}. * - * @param uploadDialogModel The tag editor model. - * - * @since 18173 (signature) + * @since xxx (signature) */ - public ChangesetManagementPanel(UploadDialogModel uploadDialogModel) { - this.uploadDialogModel = uploadDialogModel; + public ChangesetManagementPanel() { build(); - refreshGUI(); } /** + * Initializes this life cycle of the panel. + * + * @since xxx + */ + public void initLifeCycle() { + refreshChangesets(); + } + + /** + * Returns the model in use. + * @return the model + */ + public JosmComboBoxModel getModel() { + return model; + } + + /** * builds the GUI */ protected void build() { setLayout(new GridBagLayout()); + setBorder(BorderFactory.createTitledBorder(tr("Please select a changeset:"))); + GridBagConstraints gc = new GridBagConstraints(); - setBorder(BorderFactory.createEmptyBorder(3, 3, 3, 3)); - - ButtonGroup bgUseNewOrExisting = new ButtonGroup(); - - gc.gridwidth = 4; gc.gridx = 0; gc.gridy = 0; - gc.fill = GridBagConstraints.HORIZONTAL; gc.weightx = 1.0; gc.weighty = 0.0; - gc.insets = new Insets(0, 0, 5, 0); - add(new JMultilineLabel( - tr("Please decide what changeset the data is uploaded to and whether to close the changeset after the next upload.")), gc); - - gc.gridwidth = 4; - gc.gridy = 1; gc.fill = GridBagConstraints.HORIZONTAL; - gc.weightx = 1.0; - gc.weighty = 0.0; - gc.insets = new Insets(0, 0, 0, 0); - gc.anchor = GridBagConstraints.FIRST_LINE_START; - rbUseNew = new JRadioButton(tr("Upload to a new changeset")); - rbUseNew.setToolTipText(tr("Open a new changeset and use it in the next upload")); - bgUseNewOrExisting.add(rbUseNew); - add(rbUseNew, gc); + gc.anchor = GridBagConstraints.NORTHWEST; + gc.insets = new Insets(3, 3, 3, 3); - gc.gridx = 0; - gc.gridy = 2; - gc.gridwidth = 1; - gc.weightx = 0.0; - gc.fill = GridBagConstraints.HORIZONTAL; - rbExisting = new JRadioButton(tr("Upload to an existing changeset")); - rbExisting.setToolTipText(tr("Upload data to an already existing and open changeset")); - bgUseNewOrExisting.add(rbExisting); - add(rbExisting, gc); + gc.gridwidth = 3; + add(new JMultilineLabel(tr( + "Please select which changeset the data shall be uploaded to and whether to close that changeset after the next upload." + )), gc); - gc.gridx = 1; - gc.gridy = 2; gc.gridwidth = 1; - gc.weightx = 1.0; - model = new OpenChangesetComboBoxModel(); - ChangesetCache.getInstance().addChangesetCacheListener(model); + gc.gridy++; + model = new JosmComboBoxModel<>(); cbOpenChangesets = new JosmComboBox<>(model); - cbOpenChangesets.setToolTipText(tr("Select an open changeset")); + cbOpenChangesets.setToolTipText(tr("Select a changeset")); cbOpenChangesets.setRenderer(new ChangesetCellRenderer()); - cbOpenChangesets.addItemListener(new ChangesetListItemStateListener()); Dimension d = cbOpenChangesets.getPreferredSize(); d.width = 200; cbOpenChangesets.setPreferredSize(d); d.width = 100; cbOpenChangesets.setMinimumSize(d); - model.addListDataListener(this); add(cbOpenChangesets, gc); + int h = cbOpenChangesets.getPreferredSize().height; + Dimension prefSize = new Dimension(h, h); - gc.gridx = 2; - gc.gridy = 2; + gc.gridx++; gc.weightx = 0.0; - gc.gridwidth = 1; - gc.weightx = 0.0; JButton btnRefresh = new JButton(new RefreshAction()); - btnRefresh.setMargin(new Insets(0, 0, 0, 0)); + btnRefresh.setPreferredSize(prefSize); + btnRefresh.setMinimumSize(prefSize); add(btnRefresh, gc); - gc.gridx = 3; - gc.gridy = 2; - gc.gridwidth = 1; + gc.gridx++; CloseChangesetAction closeChangesetAction = new CloseChangesetAction(); - JButton btnClose = new JButton(closeChangesetAction); - btnClose.setMargin(new Insets(0, 0, 0, 0)); - cbOpenChangesets.addItemListener(closeChangesetAction); - rbExisting.addItemListener(closeChangesetAction); + btnClose = new JButton(closeChangesetAction); + btnClose.setPreferredSize(prefSize); + btnClose.setMinimumSize(prefSize); add(btnClose, gc); + gc.gridy++; gc.gridx = 0; - gc.gridy = 3; - gc.gridwidth = 4; + gc.gridwidth = 3; gc.weightx = 1.0; cbCloseAfterUpload = new JCheckBox(tr("Close changeset after upload")); cbCloseAfterUpload.setToolTipText(tr("Select to close the changeset after the next upload")); add(cbCloseAfterUpload, gc); + + cbOpenChangesets.addItemListener(this); + cbOpenChangesets.addItemListener(closeChangesetAction); + cbCloseAfterUpload.setSelected(Config.getPref().getBoolean("upload.changeset.close", true)); cbCloseAfterUpload.addItemListener(new CloseAfterUploadItemStateListener()); - rbUseNew.getModel().addItemListener(new RadioButtonHandler()); - rbExisting.getModel().addItemListener(new RadioButtonHandler()); + ChangesetCache.getInstance().addChangesetCacheListener(this); } - protected void refreshGUI() { - rbExisting.setEnabled(model.getSize() > 0); - if (model.getSize() == 0 && !rbUseNew.isSelected()) { - rbUseNew.setSelected(true); - } - cbOpenChangesets.setEnabled(model.getSize() > 0 && rbExisting.isSelected()); - } - /** * Sets the changeset to be used in the next upload + *

+ * Note: The changeset may be a new changeset that was automatically opened because the old + * changeset overflowed. In that case it was already added to the changeset cache and the + * combobox. * * @param cs the changeset + * @see UploadPrimitivesTask#handleChangesetFullResponse */ public void setSelectedChangesetForNextUpload(Changeset cs) { - int idx = model.getIndexOf(cs); - if (idx >= 0) { - rbExisting.setSelected(true); - model.setSelectedItem(cs); - } + model.setSelectedItem(cs); } /** - * Replies the currently selected changeset. null, if no changeset is - * selected or if the user has chosen to use a new changeset. + * Returns the currently selected changeset or an empty new one. * - * @return the currently selected changeset. null, if no changeset is - * selected. + * @return the currently selected changeset */ public Changeset getSelectedChangeset() { - if (rbUseNew.isSelected()) - return null; - return (Changeset) cbOpenChangesets.getSelectedItem(); + return Optional.ofNullable((Changeset) model.getSelectedItem()).orElse(new Changeset()); } /** @@ -205,36 +179,12 @@ return cbCloseAfterUpload.isSelected(); } - /* ---------------------------------------------------------------------------- */ - /* Interface ListDataListener */ - /* ---------------------------------------------------------------------------- */ - @Override - public void contentsChanged(ListDataEvent e) { - refreshGUI(); - } - - @Override - public void intervalAdded(ListDataEvent e) { - refreshGUI(); - } - - @Override - public void intervalRemoved(ListDataEvent e) { - refreshGUI(); - } - /** * Listens to changes in the selected changeset and fires property change events. */ - class ChangesetListItemStateListener implements ItemListener { - @Override - public void itemStateChanged(ItemEvent e) { - Changeset cs = (Changeset) cbOpenChangesets.getSelectedItem(); - if (cs == null) return; - if (rbExisting.isSelected()) { - firePropertyChange(SELECTED_CHANGESET_PROP, null, cs); - } - } + @Override + public void itemStateChanged(ItemEvent e) { + firePropertyChange(SELECTED_CHANGESET_PROP, null, model.getSelectedItem()); } /** @@ -260,28 +210,6 @@ } /** - * Listens to changes in the two radio buttons rbUseNew and rbUseExisting. - */ - class RadioButtonHandler implements ItemListener { - @Override - public void itemStateChanged(ItemEvent e) { - if (rbUseNew.isSelected()) { - cbOpenChangesets.setEnabled(false); - firePropertyChange(SELECTED_CHANGESET_PROP, null, null); - } else if (rbExisting.isSelected()) { - cbOpenChangesets.setEnabled(true); - if (cbOpenChangesets.getSelectedItem() == null) { - model.selectFirstChangeset(); - } - Changeset cs = (Changeset) cbOpenChangesets.getSelectedItem(); - if (cs == null) return; - uploadDialogModel.putAll(cs.getKeys()); - firePropertyChange(SELECTED_CHANGESET_PROP, null, cs); - } - } - } - - /** * Refreshes the list of open changesets */ class RefreshAction extends AbstractAction { @@ -314,11 +242,7 @@ } protected void refreshEnabledState() { - setEnabled( - cbOpenChangesets.getModel().getSize() > 0 - && cbOpenChangesets.getSelectedItem() != null - && rbExisting.isSelected() - ); + setEnabled(!getSelectedChangeset().isNew()); } @Override @@ -326,4 +250,33 @@ refreshEnabledState(); } } + + /** + * Refreshes the changesets combobox form the server. + *

+ * Note: This calls into {@link #refreshCombo} through {@link #changesetCacheUpdated} + * + * @see ChangesetCache#refreshChangesetsFromServer + */ + protected void refreshChangesets() { + try { + ChangesetCache.getInstance().refreshChangesetsFromServer(); + } catch (OsmTransferException e) { + return; + } + } + + private void refreshCombo() { + Changeset selected = (Changeset) cbOpenChangesets.getSelectedItem(); + model.removeAllElements(); + model.addElement(new Changeset()); + model.addAllElements(ChangesetCache.getInstance().getOpenChangesetsForCurrentUser()); + cbOpenChangesets.setSelectedItem(selected != null && model.getIndexOf(selected) != -1 ? selected : model.getElementAt(0)); + } + + @Override + public void changesetCacheUpdated(ChangesetCacheEvent event) { + // This listener might have been called by a background task. + SwingUtilities.invokeLater(() -> refreshCombo()); + } } Index: src/org/openstreetmap/josm/gui/io/OpenChangesetComboBoxModel.java =================================================================== --- src/org/openstreetmap/josm/gui/io/OpenChangesetComboBoxModel.java (revision 18270) +++ src/org/openstreetmap/josm/gui/io/OpenChangesetComboBoxModel.java (nonexistent) @@ -1,109 +0,0 @@ -// License: GPL. For details, see LICENSE file. -package org.openstreetmap.josm.gui.io; - -import java.util.ArrayList; -import java.util.List; - -import org.openstreetmap.josm.data.osm.Changeset; -import org.openstreetmap.josm.data.osm.ChangesetCache; -import org.openstreetmap.josm.data.osm.ChangesetCacheEvent; -import org.openstreetmap.josm.data.osm.ChangesetCacheListener; -import org.openstreetmap.josm.gui.util.GuiHelper; -import org.openstreetmap.josm.gui.widgets.JosmComboBoxModel; -import org.openstreetmap.josm.tools.Utils; - -/** - * A combobox model for the list of open changesets. The model is populated with the list - * of open changesets kept in the {@link ChangesetCache}. - * - */ -public class OpenChangesetComboBoxModel extends JosmComboBoxModel implements ChangesetCacheListener { - private final transient List changesets; - private transient Changeset selectedChangeset; - - protected Changeset getChangesetById(long id) { - return changesets.stream().filter(cs -> cs.getId() == id) - .findFirst().orElse(null); - } - - /** - * Constructs a new {@code OpenChangesetComboBoxModel}. - */ - public OpenChangesetComboBoxModel() { - this.changesets = new ArrayList<>(); - } - - /** - * Refreshes the content of the combobox model with the current list of open - * changesets from the {@link ChangesetCache}. - */ - public void refresh() { - changesets.clear(); - changesets.addAll(ChangesetCache.getInstance().getOpenChangesetsForCurrentUser()); - fireContentsChanged(this, 0, getSize()); - int idx = changesets.indexOf(selectedChangeset); - if (idx < 0) { - selectFirstChangeset(); - } else { - setSelectedItem(changesets.get(idx)); - } - } - - /** - * Selects the first changeset in the current list of open changesets - */ - public void selectFirstChangeset() { - if (Utils.isEmpty(changesets)) { - setSelectedItem(null); - } else { - setSelectedItem(changesets.get(0)); - } - } - - /* ------------------------------------------------------------------------------------ */ - /* ChangesetCacheListener */ - /* ------------------------------------------------------------------------------------ */ - @Override - public void changesetCacheUpdated(ChangesetCacheEvent event) { - GuiHelper.runInEDT(this::refresh); - } - - /* ------------------------------------------------------------------------------------ */ - /* ComboBoxModel */ - /* ------------------------------------------------------------------------------------ */ - @Override - public Changeset getElementAt(int index) { - return changesets.get(index); - } - - @Override - public int getIndexOf(Changeset anObject) { - return changesets.indexOf(anObject); - } - - @Override - public int getSize() { - return changesets.size(); - } - - @Override - public Object getSelectedItem() { - return selectedChangeset; - } - - @Override - public void setSelectedItem(Object anObject) { - if (anObject == null) { - this.selectedChangeset = null; - super.setSelectedItem(null); - return; - } - if (!(anObject instanceof Changeset)) return; - Changeset cs = (Changeset) anObject; - if (cs.getId() == 0 || !cs.isOpen()) return; - Changeset candidate = getChangesetById(cs.getId()); - if (candidate == null) return; - this.selectedChangeset = candidate; - super.setSelectedItem(selectedChangeset); - } -} Property changes on: src/org/openstreetmap/josm/gui/io/OpenChangesetComboBoxModel.java ___________________________________________________________________ Deleted: svn:eol-style ## -1 +0,0 ## -native \ No newline at end of property Index: src/org/openstreetmap/josm/gui/io/UploadDialog.java =================================================================== --- src/org/openstreetmap/josm/gui/io/UploadDialog.java (revision 18270) +++ src/org/openstreetmap/josm/gui/io/UploadDialog.java (working copy) @@ -9,6 +9,7 @@ import java.awt.Component; import java.awt.Dimension; import java.awt.FlowLayout; +import java.awt.GridBagConstraints; import java.awt.GridBagLayout; import java.awt.event.ActionEvent; import java.awt.event.WindowAdapter; @@ -23,7 +24,6 @@ import java.util.Locale; import java.util.Map; import java.util.Map.Entry; -import java.util.Optional; import java.util.stream.Collectors; import javax.swing.AbstractAction; @@ -33,7 +33,6 @@ import javax.swing.JPanel; import javax.swing.JSplitPane; import javax.swing.JTabbedPane; -import javax.swing.border.TitledBorder; import org.openstreetmap.josm.data.APIDataSet; import org.openstreetmap.josm.data.osm.Changeset; @@ -57,7 +56,6 @@ import org.openstreetmap.josm.tools.GBC; import org.openstreetmap.josm.tools.ImageProvider; import org.openstreetmap.josm.tools.InputMapUtils; -import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Utils; /** @@ -65,22 +63,21 @@ * the upload changeset and the strategy for opening/closing a changeset. * @since 2025 */ -public class UploadDialog extends AbstractUploadDialog implements PropertyChangeListener, PreferenceChangedListener { +public class UploadDialog extends AbstractUploadDialog implements PreferenceChangedListener, PropertyChangeListener { /** the unique instance of the upload dialog */ private static UploadDialog uploadDialog; /** the panel with the objects to upload */ private UploadedObjectsSummaryPanel pnlUploadedObjects; - /** the panel to select the changeset used */ - private ChangesetManagementPanel pnlChangesetManagement; + /** the "description" tab */ private BasicUploadSettingsPanel pnlBasicUploadSettings; + /** the panel to select the changeset used */ + private ChangesetManagementPanel pnlChangesetManagement; + /** the panel to select the upload strategy */ private UploadStrategySelectionPanel pnlUploadStrategySelectionPanel; - private TitledBorder tagSettingsBorder; - /** a border around the tag editor panel */ - private JPanel pnlTagEditorBorder; /** the tag editor panel */ private TagEditorPanel pnlTagEditor; /** the tabbed pane used below of the list of primitives */ @@ -96,7 +93,7 @@ /** * Constructs a new {@code UploadDialog}. */ - public UploadDialog() { + protected UploadDialog() { super(GuiHelper.getFrameForComponent(MainApplication.getMainFrame()), ModalityType.DOCUMENT_MODAL); build(); pack(); @@ -138,17 +135,17 @@ tpConfigPanels.setToolTipTextAt(0, tr("Describe the changes you made")); JPanel pnlSettings = new JPanel(new GridBagLayout()); - pnlTagEditorBorder = new JPanel(new BorderLayout()); - tagSettingsBorder = BorderFactory.createTitledBorder(tr("Tags of new changeset")); - pnlTagEditorBorder.setBorder(tagSettingsBorder); + pnlSettings.setBorder(BorderFactory.createEmptyBorder(3, 3, 3, 3)); + JPanel pnlTagEditorBorder = new JPanel(new BorderLayout()); + pnlTagEditorBorder.setBorder(BorderFactory.createTitledBorder(tr("Changeset tags:"))); pnlTagEditor = new TagEditorPanel(model, null, Changeset.MAX_CHANGESET_TAG_LENGTH); pnlTagEditorBorder.add(pnlTagEditor, BorderLayout.CENTER); - pnlChangesetManagement = new ChangesetManagementPanel(model); + pnlChangesetManagement = new ChangesetManagementPanel(); pnlUploadStrategySelectionPanel = new UploadStrategySelectionPanel(); - pnlSettings.add(pnlChangesetManagement, GBC.eop().fill(GBC.HORIZONTAL)); - pnlSettings.add(pnlUploadStrategySelectionPanel, GBC.eop().fill(GBC.HORIZONTAL)); - pnlSettings.add(pnlTagEditorBorder, GBC.eol().fill(GBC.BOTH)); + pnlSettings.add(pnlChangesetManagement, GBC.eop().fill(GridBagConstraints.HORIZONTAL)); + pnlSettings.add(pnlUploadStrategySelectionPanel, GBC.eop().fill(GridBagConstraints.HORIZONTAL)); + pnlSettings.add(pnlTagEditorBorder, GBC.eol().fill(GridBagConstraints.BOTH)); tpConfigPanels.add(pnlSettings); tpConfigPanels.setTitleAt(1, tr("Settings")); @@ -196,20 +193,16 @@ addWindowListener(new WindowEventHandler()); - // make sure the configuration panels listen to each other changes + // make sure the configuration panels listen to each others changes // + UploadParameterSummaryPanel sp = pnlBasicUploadSettings.getUploadParameterSummaryPanel(); + // the summary panel must know everything + pnlChangesetManagement.addPropertyChangeListener(sp); + pnlUploadedObjects.addPropertyChangeListener(sp); + pnlUploadStrategySelectionPanel.addPropertyChangeListener(sp); + + // update tags from selected changeset pnlChangesetManagement.addPropertyChangeListener(this); - pnlChangesetManagement.addPropertyChangeListener( - pnlBasicUploadSettings.getUploadParameterSummaryPanel() - ); - pnlUploadedObjects.addPropertyChangeListener(pnlUploadStrategySelectionPanel); - pnlUploadedObjects.addPropertyChangeListener( - pnlBasicUploadSettings.getUploadParameterSummaryPanel() - ); - pnlUploadStrategySelectionPanel.addPropertyChangeListener(this); - pnlUploadStrategySelectionPanel.addPropertyChangeListener( - pnlBasicUploadSettings.getUploadParameterSummaryPanel() - ); // users can click on either of two links in the upload parameter // summary handler. This installs the handler for these two events. @@ -241,9 +234,19 @@ Map map = new HashMap<>(); this.dataSet = dataSet; pnlBasicUploadSettings.initLifeCycle(map); + pnlChangesetManagement.initLifeCycle(); model.clear(); - model.putAll(map); - model.putAll(this.dataSet); + model.putAll(map); // init with tags from history + model.putAll(this.dataSet); // overwrite with tags from the dataset + + tpConfigPanels.setSelectedIndex(0); + pnlTagEditor.initAutoCompletion(MainApplication.getLayerManager().getEditLayer()); + pnlUploadStrategySelectionPanel.initFromPreferences(); + + // update the summary + UploadParameterSummaryPanel sumPnl = pnlBasicUploadSettings.getUploadParameterSummaryPanel(); + sumPnl.setUploadStrategySpecification(pnlUploadStrategySelectionPanel.getUploadStrategySpecification()); + sumPnl.setCloseChangesetAfterNextUpload(pnlChangesetManagement.isCloseChangesetAfterUpload()); } /** @@ -254,19 +257,24 @@ * */ public void setUploadedPrimitives(APIDataSet toUpload) { + UploadParameterSummaryPanel sumPnl = pnlBasicUploadSettings.getUploadParameterSummaryPanel(); if (toUpload == null) { if (pnlUploadedObjects != null) { List emptyList = Collections.emptyList(); pnlUploadedObjects.setUploadedPrimitives(emptyList, emptyList, emptyList); + sumPnl.setNumObjects(0); } return; } - pnlBasicUploadSettings.setUploadedPrimitives(toUpload.getPrimitives()); + List l = toUpload.getPrimitives(); + pnlBasicUploadSettings.setUploadedPrimitives(l); pnlUploadedObjects.setUploadedPrimitives( toUpload.getPrimitivesToAdd(), toUpload.getPrimitivesToUpdate(), toUpload.getPrimitivesToDelete() ); + sumPnl.setNumObjects(l.size()); + pnlUploadStrategySelectionPanel.setNumUploadedObjects(l.size()); } /** @@ -284,27 +292,13 @@ } /** - * Initializes the panel for user input - */ - public void startUserInput() { - tpConfigPanels.setSelectedIndex(0); - pnlBasicUploadSettings.startUserInput(); - pnlTagEditor.initAutoCompletion(MainApplication.getLayerManager().getEditLayer()); - pnlUploadStrategySelectionPanel.initFromPreferences(); - UploadParameterSummaryPanel pnl = pnlBasicUploadSettings.getUploadParameterSummaryPanel(); - pnl.setUploadStrategySpecification(pnlUploadStrategySelectionPanel.getUploadStrategySpecification()); - pnl.setCloseChangesetAfterNextUpload(pnlChangesetManagement.isCloseChangesetAfterUpload()); - pnl.setNumObjects(pnlUploadedObjects.getNumObjectsToUpload()); - } - - /** - * Replies the current changeset + * Returns the changeset to use complete with tags * - * @return the current changeset + * @return the changeset to use */ public Changeset getChangeset() { - Changeset cs = Optional.ofNullable(pnlChangesetManagement.getSelectedChangeset()).orElseGet(Changeset::new); - cs.setKeys(model.getTags(false)); + Changeset cs = pnlChangesetManagement.getSelectedChangeset(); + cs.setKeys(getTags(true)); return cs; } @@ -336,12 +330,12 @@ @Override public String getUploadComment() { - return model.getValue("comment"); + return model.getValue(UploadDialogModel.COMMENT); } @Override public String getUploadSource() { - return model.getValue("source"); + return model.getValue(UploadDialogModel.SOURCE); } @Override @@ -354,7 +348,6 @@ new Dimension(800, 600) ) ).applySafe(this); - startUserInput(); } else if (isShowing()) { // Avoid IllegalComponentStateException like in #8775 new WindowGeometry(this).remember(getClass().getName() + ".geometry"); } @@ -436,16 +429,14 @@ @Override public void actionPerformed(ActionEvent e) { Map tags = dialog.getTags(true); - Logging.info("Starting upload with tags {0}", tags); - /* test for empty tags in the changeset metadata and proceed only after user's confirmation. - * though, accept if key and value are empty (cf. xor). */ + // If there are empty tags in the changeset proceed only after user's confirmation. List emptyChangesetTags = new ArrayList<>(); for (final Entry i : tags.entrySet()) { final boolean isKeyEmpty = Utils.isStripEmpty(i.getKey()); final boolean isValueEmpty = Utils.isStripEmpty(i.getValue()); - final boolean ignoreKey = "comment".equals(i.getKey()) || "source".equals(i.getKey()); - if ((isKeyEmpty ^ isValueEmpty) && !ignoreKey) { + final boolean ignoreKey = UploadDialogModel.COMMENT.equals(i.getKey()) || UploadDialogModel.SOURCE.equals(i.getKey()); + if ((isKeyEmpty || isValueEmpty) && !ignoreKey) { emptyChangesetTags.add(tr("{0}={1}", i.getKey(), i.getValue())); } } @@ -527,11 +518,16 @@ @Override public void propertyChange(PropertyChangeEvent evt) { if (evt.getPropertyName().equals(ChangesetManagementPanel.SELECTED_CHANGESET_PROP)) { + // put the tags from the newly selected changeset into the model Changeset cs = (Changeset) evt.getNewValue(); - if (cs == null) { - tagSettingsBorder.setTitle(tr("Tags of new changeset")); - } else { - tagSettingsBorder.setTitle(tr("Tags of changeset {0}", cs.getId())); + if (cs != null) { + for (Map.Entry entry : cs.getKeys().entrySet()) { + String key = entry.getKey(); + // do NOT overwrite comment and source when selecting a changeset, it is + // confusing + if (!UploadDialogModel.COMMENT.equals(key) && !UploadDialogModel.SOURCE.equals(key)) + model.put(key, entry.getValue()); + } } } } Index: src/org/openstreetmap/josm/gui/io/UploadDialogModel.java =================================================================== --- src/org/openstreetmap/josm/gui/io/UploadDialogModel.java (revision 18270) +++ src/org/openstreetmap/josm/gui/io/UploadDialogModel.java (working copy) @@ -23,6 +23,10 @@ public class UploadDialogModel extends TagEditorModel { /** the "created_by" changeset OSM key */ private static final String CREATED_BY = "created_by"; + /** the "comment" changeset OSM key */ + public static final String COMMENT = "comment"; + /** the "source" changeset OSM key */ + public static final String SOURCE = "source"; /** the user-agent */ private final String agent = Version.getInstance().getAgentString(false); /** whether to extract hashtags from comment */ @@ -38,7 +42,7 @@ locked = true; // add "hashtags" if any if (hashtags) { - put("hashtags", findHashTags(getValue("comment"))); + put("hashtags", findHashTags(getValue(COMMENT))); } // add/update "created_by" final String createdBy = getValue(CREATED_BY); @@ -71,12 +75,12 @@ * @return the hashtags separated by ";" or null */ String findHashTags(String comment) { - String hashtags = String.join(";", + String hashTags = String.join(";", Arrays.stream(comment.split("\\s", -1)) .map(s -> Utils.strip(s, ",;")) .filter(s -> s.matches("#[a-zA-Z0-9][-_a-zA-Z0-9]+")) .collect(Collectors.toList())); - return hashtags.isEmpty() ? null : hashtags; + return hashTags.isEmpty() ? null : hashTags; } /** @@ -115,9 +119,11 @@ List l = tags.stream().filter(tm -> tm.getName().equals(key)).collect(Collectors.toList()); if (!l.isEmpty()) { if (value != null) - l.get(0).setValue(value); + for (TagModel tm : l) { + tm.setValue(value); + } else - tags.remove(l.get(0)); + tags.removeIf(tm -> tm.getName().equals(key)); } else if (value != null) { tags.add(new TagModel(key, value)); } @@ -162,7 +168,7 @@ public void putAll(DataSet dataSet) { if (dataSet != null) { putAll(dataSet.getChangeSetTags()); - put("comment", addHashTagsFromDataSet(getValue("comment"), dataSet)); + put(COMMENT, addHashTagsFromDataSet(getValue(COMMENT), dataSet)); } } } Index: src/org/openstreetmap/josm/gui/io/UploadParameterSummaryPanel.java =================================================================== --- src/org/openstreetmap/josm/gui/io/UploadParameterSummaryPanel.java (revision 18270) +++ src/org/openstreetmap/josm/gui/io/UploadParameterSummaryPanel.java (working copy) @@ -53,7 +53,7 @@ if (selectedChangeset == null || selectedChangeset.isNew()) { return tr("Objects are uploaded to a new changeset."); } else { - return tr("Objects are uploaded to the open changeset {0} with upload comment ''{1}''.", + return tr("Objects are uploaded to the open changeset {0} ''{1}''.", selectedChangeset.getId(), selectedChangeset.getComment() ); @@ -199,9 +199,6 @@ } else if (evt.getPropertyName().equals(ChangesetManagementPanel.CLOSE_CHANGESET_AFTER_UPLOAD)) { closeChangesetAfterNextUpload = (Boolean) evt.getNewValue(); updateSummary(); - } else if (evt.getPropertyName().equals(UploadedObjectsSummaryPanel.NUM_OBJECTS_TO_UPLOAD_PROP)) { - numObjects = (Integer) evt.getNewValue(); - updateSummary(); } else if (evt.getPropertyName().equals(UploadStrategySelectionPanel.UPLOAD_STRATEGY_SPECIFICATION_PROP)) { this.spec = (UploadStrategySpecification) evt.getNewValue(); updateSummary(); Index: src/org/openstreetmap/josm/gui/io/UploadPrimitivesTask.java =================================================================== --- src/org/openstreetmap/josm/gui/io/UploadPrimitivesTask.java (revision 18270) +++ src/org/openstreetmap/josm/gui/io/UploadPrimitivesTask.java (working copy) @@ -82,7 +82,12 @@ this.processedPrimitives = new HashSet<>(); } - protected MaxChangesetSizeExceededPolicy askMaxChangesetSizeExceedsPolicy() { + /** + * Prompt the user about how to proceed. + * + * @return the policy selected by the user + */ + protected MaxChangesetSizeExceededPolicy promptUserForPolicy() { ButtonSpec[] specs = { new ButtonSpec( tr("Continue uploading"), @@ -145,29 +150,29 @@ } /** - * Opens a new changeset. + * Handles a server changeset full response. + *

+ * Handles a server changeset full response by either aborting or opening a new changeset, if the + * user requested it so. + * + * @return true if the upload process should continue with the new changeset, false if the + * upload should be interrupted + * @throws OsmTransferException "if something goes wrong." */ - protected void openNewChangeset() { - // make sure the current changeset is removed from the upload dialog. - ChangesetCache.getInstance().update(changeset); - Changeset newChangeSet = new Changeset(); - newChangeSet.setKeys(this.changeset.getKeys()); - this.changeset = newChangeSet; - } - - protected boolean recoverFromChangesetFullException() throws OsmTransferException { - if (toUpload.getSize() - processedPrimitives.size() == 0) { + protected boolean handleChangesetFullResponse() throws OsmTransferException { + if (processedPrimitives.size() == toUpload.getSize()) { strategy.setPolicy(MaxChangesetSizeExceededPolicy.ABORT); return false; } if (strategy.getPolicy() == null || strategy.getPolicy() == MaxChangesetSizeExceededPolicy.ABORT) { - strategy.setPolicy(askMaxChangesetSizeExceedsPolicy()); + strategy.setPolicy(promptUserForPolicy()); } switch(strategy.getPolicy()) { case AUTOMATICALLY_OPEN_NEW_CHANGESETS: - // prepare the state of the task for a next iteration in uploading. - closeChangesetIfRequired(); - openNewChangeset(); + Changeset newChangeSet = new Changeset(); + newChangeSet.setKeys(changeset.getKeys()); + closeChangeset(); + this.changeset = newChangeSet; toUpload.removeProcessed(processedPrimitives); return true; case ABORT: @@ -259,7 +264,8 @@ writer = new OsmServerWriter(); } writer.uploadOsm(strategy, toUpload.getPrimitives(), changeset, getProgressMonitor().createSubTaskMonitor(1, false)); - + // If the changeset was new, now it is open. + ChangesetCache.getInstance().update(changeset); // if we get here we've successfully uploaded the data. Exit the loop. break; } catch (OsmTransferCanceledException e) { @@ -276,15 +282,15 @@ switch(e.getSource()) { case UPLOAD_DATA: // Most likely the changeset is full. Try to recover and continue - // with a new changeset, but let the user decide first (see - // recoverFromChangesetFullException) - if (recoverFromChangesetFullException()) { + // with a new changeset, but let the user decide first. + if (handleChangesetFullResponse()) { continue; } lastException = e; break uploadloop; + case UPDATE_CHANGESET: + case CLOSE_CHANGESET: case UNSPECIFIED: - case UPDATE_CHANGESET: default: // The changeset was closed when we tried to update it. Probably, our // local list of open changesets got out of sync with the server state. @@ -291,6 +297,7 @@ // The user will have to select another open changeset. // Rethrow exception - this will be handled later. changeset.setOpen(false); + ChangesetCache.getInstance().update(changeset); throw e; } } finally { @@ -319,20 +326,44 @@ cleanupAfterUpload(); } + /** + * Closes the changeset on the server and locally. + * + * @throws OsmTransferException "if something goes wrong." + */ + private void closeChangeset() throws OsmTransferException { + if (changeset != null && !changeset.isNew() && changeset.isOpen()) { + // CHECKSTYLE.OFF: EmptyBlock + try { + OsmApi.getOsmApi().closeChangeset(changeset, progressMonitor.createSubTaskMonitor(0, false)); + } catch (ChangesetClosedException e) { + // Do not raise a stink, probably the changeset timed out. + } finally { + changeset.setOpen(false); + ChangesetCache.getInstance().update(changeset); + } + // CHECKSTYLE.ON: EmptyBlock + } + } + private void closeChangesetIfRequired() throws OsmTransferException { - if (strategy.isCloseChangesetAfterUpload() && changeset != null && !changeset.isNew() && changeset.isOpen()) { - OsmApi.getOsmApi().closeChangeset(changeset, progressMonitor.createSubTaskMonitor(0, false)); + if (strategy.isCloseChangesetAfterUpload()) { + closeChangeset(); } } - @Override protected void finish() { - - // depending on the success of the upload operation and on the policy for - // multi changeset uploads this will sent the user back to the appropriate - // place in JOSM, either - // - to an error dialog - // - to the Upload Dialog - // - to map editing + /** + * Depending on the success of the upload operation and on the policy for + * multi changeset uploads this will send the user back to the appropriate + * place in JOSM, either: + *

+ */ + @Override + protected void finish() { GuiHelper.runInEDT(() -> { // if the changeset is still open after this upload we want it to be selected on the next upload ChangesetCache.getInstance().update(changeset); Index: src/org/openstreetmap/josm/gui/io/UploadStrategySelectionPanel.java =================================================================== --- src/org/openstreetmap/josm/gui/io/UploadStrategySelectionPanel.java (revision 18270) +++ src/org/openstreetmap/josm/gui/io/UploadStrategySelectionPanel.java (working copy) @@ -10,12 +10,10 @@ import java.awt.Insets; import java.awt.event.ActionEvent; import java.awt.event.ActionListener; -import java.awt.event.FocusAdapter; import java.awt.event.FocusEvent; +import java.awt.event.FocusListener; import java.awt.event.ItemEvent; import java.awt.event.ItemListener; -import java.beans.PropertyChangeEvent; -import java.beans.PropertyChangeListener; import java.util.EnumMap; import java.util.Map; import java.util.Map.Entry; @@ -44,7 +42,7 @@ * Clients can listen for property change events for the property * {@link #UPLOAD_STRATEGY_SPECIFICATION_PROP}. */ -public class UploadStrategySelectionPanel extends JPanel implements PropertyChangeListener { +public class UploadStrategySelectionPanel extends JPanel { /** * The property for the upload strategy @@ -56,10 +54,8 @@ private transient Map lblNumRequests; private final JosmTextField tfChunkSize = new JosmTextField(4); private final JPanel pnlMultiChangesetPolicyPanel = new JPanel(new GridBagLayout()); - private final JRadioButton rbFillOneChangeset = new JRadioButton( - tr("Fill up one changeset and return to the Upload Dialog")); - private final JRadioButton rbUseMultipleChangesets = new JRadioButton( - tr("Open and use as many new changesets as necessary")); + private final JRadioButton rbFillOneChangeset = new JRadioButton(); + private final JRadioButton rbUseMultipleChangesets = new JRadioButton(); private JMultilineLabel lblMultiChangesetPoliciesHeader; private long numUploadedObjects; @@ -73,7 +69,7 @@ protected JPanel buildUploadStrategyPanel() { JPanel pnl = new JPanel(new GridBagLayout()); - pnl.setBorder(BorderFactory.createTitledBorder(tr("Please select the upload strategy:"))); + pnl.setBorder(BorderFactory.createTitledBorder(tr("Please select an upload strategy:"))); ButtonGroup bgStrategies = new ButtonGroup(); rbStrategy = new EnumMap<>(UploadStrategy.class); lblNumRequests = new EnumMap<>(UploadStrategy.class); @@ -91,55 +87,36 @@ gc.weighty = 0.0; gc.gridwidth = 1; gc.fill = GridBagConstraints.HORIZONTAL; - gc.insets = new Insets(0, 0, 3, 0); + gc.insets = new Insets(3, 3, 3, 3); gc.anchor = GridBagConstraints.FIRST_LINE_START; JRadioButton radioButton = rbStrategy.get(UploadStrategy.SINGLE_REQUEST_STRATEGY); - radioButton.setText(tr("Upload data in one request")); + radioButton.setText(tr("Upload all objects in one request")); pnl.add(radioButton, gc); - gc.gridx = 3; - gc.gridy = 1; + gc.gridx = 2; gc.weightx = 1.0; - gc.weighty = 0.0; - gc.gridwidth = 1; pnl.add(lblNumRequests.get(UploadStrategy.SINGLE_REQUEST_STRATEGY), gc); // -- chunked dataset strategy + gc.gridy++; gc.gridx = 0; - gc.gridy = 2; gc.weightx = 0.0; - gc.weighty = 0.0; radioButton = rbStrategy.get(UploadStrategy.CHUNKED_DATASET_STRATEGY); - radioButton.setText(tr("Upload data in chunks of objects. Chunk size: ")); + radioButton.setText(tr("Upload objects in chunks of size: ")); pnl.add(radioButton, gc); + gc.gridx = 1; + pnl.add(tfChunkSize, gc); gc.gridx = 2; - gc.gridy = 2; - gc.weightx = 0.0; - gc.weighty = 0.0; - gc.gridwidth = 1; - pnl.add(tfChunkSize, gc); - gc.gridx = 3; - gc.gridy = 2; - gc.weightx = 0.0; - gc.weighty = 0.0; - gc.gridwidth = 1; pnl.add(lblNumRequests.get(UploadStrategy.CHUNKED_DATASET_STRATEGY), gc); // -- single request strategy + gc.gridy++; gc.gridx = 0; - gc.gridy = 3; - gc.weightx = 0.0; - gc.weighty = 0.0; radioButton = rbStrategy.get(UploadStrategy.INDIVIDUAL_OBJECTS_STRATEGY); radioButton.setText(tr("Upload each object individually")); pnl.add(radioButton, gc); - gc.gridx = 3; - gc.gridy = 3; - gc.weightx = 0.0; - gc.weighty = 0.0; - gc.gridwidth = 1; + gc.gridx = 2; pnl.add(lblNumRequests.get(UploadStrategy.INDIVIDUAL_OBJECTS_STRATEGY), gc); - tfChunkSize.addFocusListener(new TextFieldFocusHandler()); new ChunkSizeValidator(tfChunkSize); StrategyChangeListener strategyChangeListener = new StrategyChangeListener(); @@ -158,15 +135,18 @@ gc.gridy = 0; gc.fill = GridBagConstraints.HORIZONTAL; gc.anchor = GridBagConstraints.FIRST_LINE_START; + gc.insets = new Insets(3, 3, 3, 3); gc.weightx = 1.0; lblMultiChangesetPoliciesHeader = new JMultilineLabel( - tr("There are multiple changesets necessary in order to upload {0} objects. " + - "Which strategy do you want to use?", + tr("Multiple changesets are necessary to upload {0} objects. " + + "Please select a strategy:", numUploadedObjects)); pnlMultiChangesetPolicyPanel.add(lblMultiChangesetPoliciesHeader, gc); - gc.gridy = 1; + gc.gridy++; + rbFillOneChangeset.setText(tr("Fill up one changeset and return to the Upload Dialog")); pnlMultiChangesetPolicyPanel.add(rbFillOneChangeset, gc); - gc.gridy = 2; + gc.gridy++; + rbUseMultipleChangesets.setText(tr("Open and use as many new changesets as necessary")); pnlMultiChangesetPolicyPanel.add(rbUseMultipleChangesets, gc); ButtonGroup bgMultiChangesetPolicies = new ButtonGroup(); @@ -184,19 +164,11 @@ gc.weightx = 1.0; gc.weighty = 0.0; gc.anchor = GridBagConstraints.NORTHWEST; - gc.insets = new Insets(3, 3, 3, 3); add(buildUploadStrategyPanel(), gc); gc.gridy = 1; add(buildMultiChangesetPolicyPanel(), gc); - // consume remaining space - gc.gridy = 2; - gc.fill = GridBagConstraints.BOTH; - gc.weightx = 1.0; - gc.weighty = 1.0; - add(new JPanel(), gc); - Capabilities capabilities = OsmApi.getOsmApi().getCapabilities(); int maxChunkSize = capabilities != null ? capabilities.getMaxChangesetSize() : -1; pnlMultiChangesetPolicyPanel.setVisible( @@ -312,7 +284,7 @@ if (maxChunkSize > 0 && numUploadedObjects > maxChunkSize) { rbStrategy.get(UploadStrategy.SINGLE_REQUEST_STRATEGY).setEnabled(false); JRadioButton lbl = rbStrategy.get(UploadStrategy.SINGLE_REQUEST_STRATEGY); - lbl.setText(tr("Upload in one request not possible (too many objects to upload)")); + lbl.setEnabled(false); lbl.setToolTipText(tr("Cannot upload {0} objects in one request because the
" + "max. changeset size {1} on server ''{2}'' is exceeded.", numUploadedObjects, maxChunkSize, OsmApi.getOsmApi().getBaseUrl() @@ -333,7 +305,7 @@ } else { rbStrategy.get(UploadStrategy.SINGLE_REQUEST_STRATEGY).setEnabled(true); JRadioButton lbl = rbStrategy.get(UploadStrategy.SINGLE_REQUEST_STRATEGY); - lbl.setText(tr("Upload data in one request")); + lbl.setEnabled(true); lbl.setToolTipText(null); lblNumRequests.get(UploadStrategy.SINGLE_REQUEST_STRATEGY).setVisible(true); @@ -368,24 +340,6 @@ tfChunkSize.requestFocusInWindow(); } - @Override - public void propertyChange(PropertyChangeEvent evt) { - if (evt.getPropertyName().equals(UploadedObjectsSummaryPanel.NUM_OBJECTS_TO_UPLOAD_PROP)) { - setNumUploadedObjects((Integer) evt.getNewValue()); - } - } - - static class TextFieldFocusHandler extends FocusAdapter { - @Override - public void focusGained(FocusEvent e) { - Component c = e.getComponent(); - if (c instanceof JosmTextField) { - JosmTextField tf = (JosmTextField) c; - tf.selectAll(); - } - } - } - class ChunkSizeValidator extends AbstractTextComponentValidator { ChunkSizeValidator(JTextComponent tc) { super(tc); @@ -423,7 +377,7 @@ } } - class StrategyChangeListener extends FocusAdapter implements ItemListener, ActionListener { + class StrategyChangeListener implements FocusListener, ItemListener, ActionListener { protected void notifyStrategy() { firePropertyChange(UPLOAD_STRATEGY_SPECIFICATION_PROP, null, getUploadStrategySpecification()); @@ -446,6 +400,15 @@ } @Override + public void focusGained(FocusEvent e) { + Component c = e.getComponent(); + if (c instanceof JosmTextField) { + JosmTextField tf = (JosmTextField) c; + tf.selectAll(); + } + } + + @Override public void focusLost(FocusEvent e) { notifyStrategy(); } Index: src/org/openstreetmap/josm/gui/io/UploadedObjectsSummaryPanel.java =================================================================== --- src/org/openstreetmap/josm/gui/io/UploadedObjectsSummaryPanel.java (revision 18270) +++ src/org/openstreetmap/josm/gui/io/UploadedObjectsSummaryPanel.java (working copy) @@ -28,11 +28,6 @@ * @since 2599 */ public class UploadedObjectsSummaryPanel extends JPanel { - /** - * The swing property name for the number of objects to upload - */ - public static final String NUM_OBJECTS_TO_UPLOAD_PROP = UploadedObjectsSummaryPanel.class.getName() + ".numObjectsToUpload"; - /** the list with the added primitives */ private PrimitiveList lstAdd; private JLabel lblAdd; @@ -145,8 +140,7 @@ gcList.gridy = y; add(spDelete, gcList); } - - firePropertyChange(NUM_OBJECTS_TO_UPLOAD_PROP, 0, getNumObjectsToUpload()); + revalidate(); } /** Index: src/org/openstreetmap/josm/gui/tagging/TagEditorModel.java =================================================================== --- src/org/openstreetmap/josm/gui/tagging/TagEditorModel.java (revision 18270) +++ src/org/openstreetmap/josm/gui/tagging/TagEditorModel.java (working copy) @@ -30,6 +30,7 @@ import org.openstreetmap.josm.data.osm.Tagged; import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetType; import org.openstreetmap.josm.tools.CheckParameterUtil; +import org.openstreetmap.josm.tools.Utils; /** * TagEditorModel is a table model to use with {@link TagEditorPanel}. @@ -53,7 +54,7 @@ private transient OsmPrimitive primitive; - private EndEditListener endEditListener; + private transient EndEditListener endEditListener; /** * Creates a new tag editor model. Internally allocates two selection models @@ -278,8 +279,6 @@ * @param tagIndices a list of tag indices */ public void deleteTagNames(int... tagIndices) { - if (tags == null) - return; commitPendingEdit(); for (int tagIdx : tagIndices) { TagModel tag = tags.get(tagIdx); @@ -297,8 +296,6 @@ * @param tagIndices the lit of tag indices */ public void deleteTagValues(int... tagIndices) { - if (tags == null) - return; commitPendingEdit(); for (int tagIdx : tagIndices) { TagModel tag = tags.get(tagIdx); @@ -332,8 +329,6 @@ * @param tagIndices the list of tag indices */ public void deleteTags(int... tagIndices) { - if (tags == null) - return; commitPendingEdit(); List toDelete = Arrays.stream(tagIndices).mapToObj(tags::get).filter(Objects::nonNull).collect(Collectors.toList()); toDelete.forEach(tags::remove); @@ -441,12 +436,19 @@ if (tag.getValueCount() > 1) { continue; } + boolean isKeyEmpty = Utils.isStripEmpty(tag.getName()); + boolean isValueEmpty = Utils.isStripEmpty(tag.getValue()); + // just the empty line at the bottom of the JTable + if (isKeyEmpty && isValueEmpty) { + continue; + } + // tag name holds an empty key. Don't apply it to the selection. - if (!keepEmpty && (tag.getName().trim().isEmpty() || tag.getValue().trim().isEmpty())) { + if (!keepEmpty && (isKeyEmpty || isValueEmpty)) { continue; } - result.put(tag.getName().trim(), tag.getValue().trim()); + result.put(Utils.strip(tag.getName()), Utils.strip(tag.getValue())); } return result; } @@ -496,7 +498,7 @@ // tag name holds an empty key. Don't apply it to the selection. // - if (tag.getName().trim().isEmpty()) + if (Utils.isStripEmpty(tag.getName())) return null; return new ChangePropertyCommand(primitives, tag.getName(), tag.getValue()); @@ -528,7 +530,7 @@ */ public List getKeys() { return tags.stream() - .filter(tag -> !tag.getName().trim().isEmpty()) + .filter(tag -> !Utils.isStripEmpty(tag.getName())) .map(TagModel::getName) .collect(Collectors.toList()); } Index: src/org/openstreetmap/josm/gui/tagging/TagTable.java =================================================================== --- src/org/openstreetmap/josm/gui/tagging/TagTable.java (revision 18270) +++ src/org/openstreetmap/josm/gui/tagging/TagTable.java (working copy) @@ -35,6 +35,7 @@ import org.openstreetmap.josm.gui.widgets.JosmTable; import org.openstreetmap.josm.tools.ImageProvider; import org.openstreetmap.josm.tools.Logging; +import org.openstreetmap.josm.tools.Utils; /** * This is the tabular editor component for OSM tags. @@ -88,7 +89,7 @@ } else if (col == 1 && row == getRowCount()-1) { // we are at the end. Append an empty row and move the focus to its second column String key = ((TagModel) model.getValueAt(row, 0)).getName(); - if (!key.trim().isEmpty()) { + if (!Utils.isStripEmpty(key)) { model.appendNewTag(); col = 0; row++; @@ -242,7 +243,7 @@ cEditor.stopCellEditing(); } final int rowIdx = model.getRowCount()-1; - if (rowIdx < 0 || !((TagModel) model.getValueAt(rowIdx, 0)).getName().trim().isEmpty()) { + if (rowIdx < 0 || !Utils.isStripEmpty(((TagModel) model.getValueAt(rowIdx, 0)).getName())) { model.appendNewTag(); } requestFocusInCell(model.getRowCount()-1, 0); Index: src/org/openstreetmap/josm/gui/widgets/JosmComboBoxModel.java =================================================================== --- src/org/openstreetmap/josm/gui/widgets/JosmComboBoxModel.java (revision 18270) +++ src/org/openstreetmap/josm/gui/widgets/JosmComboBoxModel.java (working copy) @@ -54,12 +54,19 @@ * {@link javax.swing.DefaultComboBoxModel}. * * @param element the element to get the index of - * @return an int representing the index position, where 0 is the first position + * @return the index of the first occurrence of the specified element in this model, + * or -1 if this model does not contain the element */ public int getIndexOf(E element) { return elements.indexOf(element); } + protected void doAddElement(E element) { + if (element != null && (maxSize == -1 || getSize() < maxSize)) { + elements.add(element); + } + } + // // interface java.lang.Iterable // @@ -78,14 +85,22 @@ */ @Override public void addElement(E element) { - if (element != null && (maxSize == -1 || getSize() < maxSize)) { - elements.add(element); - } + doAddElement(element); + fireIntervalAdded(this, elements.size() - 1, elements.size() - 1); } @Override public void removeElement(Object elem) { - elements.remove(elem); + int index = elements.indexOf(elem); + if (elem == selected) { + if (index == 0) { + setSelectedItem(getSize() == 1 ? null : getElementAt(index + 1)); + } else { + setSelectedItem(getElementAt(index - 1)); + } + } + if (elements.remove(elem)) + fireIntervalRemoved(this, index, index); } @Override @@ -114,6 +129,7 @@ removeElementAt(getSize() - 1); } elements.add(index, element); + fireIntervalAdded(this, index, index); } // @@ -166,7 +182,11 @@ * @param elems The elements to add. */ public void addAllElements(Collection elems) { - elems.forEach(e -> addElement(e)); + int index0 = elements.size(); + elems.forEach(e -> doAddElement(e)); + int index1 = elements.size() - 1; + if (index0 <= index1) + fireIntervalAdded(this, index0, index1); } /** @@ -177,7 +197,11 @@ * {@code String}. */ public void addAllElements(Collection strings, Function buildE) { - strings.forEach(s -> addElement(buildE.apply(s))); + int index0 = elements.size(); + strings.forEach(s -> doAddElement(buildE.apply(s))); + int index1 = elements.size() - 1; + if (index0 <= index1) + fireIntervalAdded(this, index0, index1); } /** @@ -204,11 +228,10 @@ */ public void removeAllElements() { if (!elements.isEmpty()) { - int firstIndex = 0; int lastIndex = elements.size() - 1; elements.clear(); selected = null; - fireIntervalRemoved(this, firstIndex, lastIndex); + fireIntervalRemoved(this, 0, lastIndex); } else { selected = null; } Property changes on: src/org/openstreetmap/josm/gui/widgets/JosmComboBoxModel.java ___________________________________________________________________ Deleted: svn:eol-style ## -1 +0,0 ## -native \ No newline at end of property Index: src/org/openstreetmap/josm/io/ChangesetClosedException.java =================================================================== --- src/org/openstreetmap/josm/io/ChangesetClosedException.java (revision 18270) +++ src/org/openstreetmap/josm/io/ChangesetClosedException.java (working copy) @@ -43,6 +43,11 @@ */ UPLOAD_DATA, /** + * The exception was thrown when we tried to close a changeset. Probably the changeset + * already timed out on the server. + */ + CLOSE_CHANGESET, + /** * Unspecified source */ UNSPECIFIED Index: src/org/openstreetmap/josm/io/OsmApi.java =================================================================== --- src/org/openstreetmap/josm/io/OsmApi.java (revision 18270) +++ src/org/openstreetmap/josm/io/OsmApi.java (working copy) @@ -387,6 +387,9 @@ ((OsmPrimitive) osm).getDataSet().lock(); } } + } catch (ChangesetClosedException e) { + e.setSource(ChangesetClosedException.Source.UPDATE_CHANGESET); + throw e; } catch (NumberFormatException e) { throw new OsmTransferException(errHandler.apply(ret), e); } @@ -528,8 +531,11 @@ initialize(monitor); // send "\r\n" instead of empty string, so we don't send zero payload - workaround bugs in proxy software sendPutRequest("changeset/" + changeset.getId() + "/close", "\r\n", monitor); + } catch (ChangesetClosedException e) { + e.setSource(ChangesetClosedException.Source.CLOSE_CHANGESET); + throw e; + } finally { changeset.setOpen(false); - } finally { monitor.finishTask(); } } @@ -564,9 +570,8 @@ public Collection uploadDiff(Collection list, ProgressMonitor monitor) throws OsmTransferException { try { + ensureValidChangeset(); monitor.beginTask("", list.size() * 2); - if (changeset == null) - throw new OsmTransferException(tr("No changeset present for diff upload.")); initialize(monitor); @@ -593,7 +598,8 @@ getChangeset(), monitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false) ); - } catch (OsmTransferException e) { + } catch (ChangesetClosedException e) { + e.setSource(ChangesetClosedException.Source.UPLOAD_DATA); throw e; } catch (XmlParsingException e) { throw new OsmTransferException(e); @@ -751,7 +757,7 @@ throw new OsmApiPrimitiveGoneException(errorHeader, errorBody); case HttpURLConnection.HTTP_CONFLICT: if (ChangesetClosedException.errorHeaderMatchesPattern(errorHeader)) - throw new ChangesetClosedException(errorBody, ChangesetClosedException.Source.UPLOAD_DATA); + throw new ChangesetClosedException(errorBody, ChangesetClosedException.Source.UNSPECIFIED); else throw new OsmApiException(retCode, errorHeader, errorBody); case HttpURLConnection.HTTP_UNAUTHORIZED: Index: src/org/openstreetmap/josm/io/UploadStrategySpecification.java =================================================================== --- src/org/openstreetmap/josm/io/UploadStrategySpecification.java (revision 18270) +++ src/org/openstreetmap/josm/io/UploadStrategySpecification.java (working copy) @@ -150,6 +150,12 @@ } @Override + public String toString() { + return String.format("Strategy: %s, ChunkSize: %d, Policy: %s, Close after: %b", + strategy.toString(), chunkSize, policy == null ? "none" : policy.toString(), closeChangesetAfterUpload); + } + + @Override public int hashCode() { return Objects.hash(strategy, chunkSize, policy, closeChangesetAfterUpload); } Index: test/unit/org/openstreetmap/josm/data/osm/ChangesetCacheTest.java =================================================================== --- test/unit/org/openstreetmap/josm/data/osm/ChangesetCacheTest.java (revision 18270) +++ test/unit/org/openstreetmap/josm/data/osm/ChangesetCacheTest.java (working copy) @@ -18,6 +18,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; import org.openstreetmap.josm.data.UserIdentityManager; import org.openstreetmap.josm.testutils.JOSMTestRules; +import org.openstreetmap.josm.testutils.annotations.BasicPreferences; import org.openstreetmap.josm.tools.Logging; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -25,6 +26,7 @@ /** * Unit test of {@link ChangesetCache} */ +@BasicPreferences class ChangesetCacheTest { /** Index: test/unit/org/openstreetmap/josm/gui/io/ChangesetManagementPanelTest.java =================================================================== --- test/unit/org/openstreetmap/josm/gui/io/ChangesetManagementPanelTest.java (revision 18270) +++ test/unit/org/openstreetmap/josm/gui/io/ChangesetManagementPanelTest.java (working copy) @@ -17,6 +17,6 @@ */ @Test void testChangesetManagementPanel() { - assertNotNull(new ChangesetManagementPanel(new UploadDialogModel())); + assertNotNull(new ChangesetManagementPanel()); } }