Ticket #21215: 21215.patch

File 21215.patch, 3.9 KB (added by taylor.smock, 4 years ago)

Non-regression test, avoid creating new AutoCompletionItem where possible, fix StackOverflowError

  • src/org/openstreetmap/josm/gui/widgets/ComboBoxHistory.java

    diff --git a/src/org/openstreetmap/josm/gui/widgets/ComboBoxHistory.java b/src/org/openstreetmap/josm/gui/widgets/ComboBoxHistory.java
    index e1238016a6..f7a70a7cf4 100644
    a b class ComboBoxHistory extends DefaultComboBoxModel<AutoCompletionItem> implement  
    4141    public void addElement(AutoCompletionItem o) {
    4242        String newEntry = o.getValue();
    4343
     44        boolean alreadyAdded = false;
    4445        // if history contains this object already, delete it,
    4546        // so that it looks like a move to the top
    4647        for (int i = 0; i < getSize(); i++) {
    4748            String oldEntry = getElementAt(i).getValue();
    4849            if (oldEntry.equals(newEntry)) {
    49                 removeElementAt(i);
     50                if (i == 0) {
     51                    alreadyAdded = true;
     52                    break;
     53                } else {
     54                    removeElementAt(i);
     55                }
    5056            }
    5157        }
    5258
    53         // insert element at the top
    54         insertElementAt(o, 0);
     59        if (!alreadyAdded) {
     60            // insert element at the top
     61            insertElementAt(o, 0);
     62        }
    5563
    5664        // remove an element, if the history gets too large
    5765        if (getSize() > maxSize) {
  • src/org/openstreetmap/josm/gui/widgets/HistoryComboBox.java

    diff --git a/src/org/openstreetmap/josm/gui/widgets/HistoryComboBox.java b/src/org/openstreetmap/josm/gui/widgets/HistoryComboBox.java
    index dc6955ece1..33de5a32ac 100644
    a b import java.util.List;  
    55
    66import javax.swing.text.JTextComponent;
    77
     8import org.openstreetmap.josm.data.tagging.ac.AutoCompletionItem;
    89import org.openstreetmap.josm.gui.tagging.ac.AutoCompletingComboBox;
    910import org.openstreetmap.josm.spi.preferences.Config;
    1011
    public class HistoryComboBox extends AutoCompletingComboBox {  
    5455     * @see ComboBoxHistory#addElement(java.lang.String)
    5556     */
    5657    public void addCurrentItemToHistory() {
    57         model.addElement(getEditor().getItem().toString());
     58        Object item = getEditor().getItem();
     59        // This avoids instantiating multiple AutoCompletionItems
     60        if (item instanceof AutoCompletionItem) {
     61            model.addElement((AutoCompletionItem) item);
     62        } else {
     63            model.addElement(item.toString());
     64        }
    5865    }
    5966
    6067    /**
  • test/unit/org/openstreetmap/josm/gui/widgets/HistoryComboBoxTest.java

    diff --git a/test/unit/org/openstreetmap/josm/gui/widgets/HistoryComboBoxTest.java b/test/unit/org/openstreetmap/josm/gui/widgets/HistoryComboBoxTest.java
    index 6475b5d1bd..dcdae47eb2 100644
    a b class HistoryComboBoxTest {  
    4444        historyComboBox.getEditor().setItem(null);
    4545        assertDoesNotThrow(historyComboBox::addCurrentItemToHistory);
    4646    }
     47
     48    /**
     49     * Non-regression test for JOSM #21215: StackOverflowError
     50     */
     51    @Test
     52    void testNonRegression21215() {
     53        final HistoryComboBox historyComboBox = new HistoryComboBox();
     54        // utils plugin2 added a listener that pretty much did this
     55        historyComboBox.addItemListener(event -> historyComboBox.addCurrentItemToHistory());
     56        final AutoCompletionItem testItem = new AutoCompletionItem("testNonRegression21215");
     57        // Add the original item
     58        historyComboBox.getEditor().setItem(testItem);
     59        historyComboBox.addCurrentItemToHistory();
     60
     61        // add a new item
     62        historyComboBox.getEditor().setItem(new AutoCompletionItem("testNonRegression21215_2"));
     63        historyComboBox.addCurrentItemToHistory();
     64
     65        // Readd the first item
     66        historyComboBox.getEditor().setItem(testItem);
     67        assertDoesNotThrow(historyComboBox::addCurrentItemToHistory);
     68    }
    4769}