Skip to content

Commit

Permalink
fix #21476 - Fix and add non-regression test for IllegalStateExceptio…
Browse files Browse the repository at this point in the history
…n in ChangesetQuery.forCurrentUser when the current user is anonymous (patch by taylor.smock)

git-svn-id: https://josm.openstreetmap.de/svn/trunk@18299 0c6e7542-c601-0410-84e7-c038aed88b3b
  • Loading branch information
don-vip committed Oct 31, 2021
1 parent 9abe06f commit 242640a
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 2 deletions.
3 changes: 2 additions & 1 deletion src/org/openstreetmap/josm/data/osm/ChangesetCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -273,7 +274,7 @@ public void refreshChangesetsFromServer() throws OsmTransferException {
synchronized (this) {
reader = new OsmServerChangesetReader();
}
List<Changeset> server = reader.queryChangesets(ChangesetQuery.forCurrentUser().beingOpen(true), null);
List<Changeset> server = UserIdentityManager.getInstance().isAnonymous() ? Collections.emptyList() : reader.queryChangesets(ChangesetQuery.forCurrentUser().beingOpen(true), null);
Logging.info("{0} open changesets on server", server.size());

DefaultChangesetCacheEvent e = new DefaultChangesetCacheEvent(this);
Expand Down
2 changes: 1 addition & 1 deletion src/org/openstreetmap/josm/io/UploadStrategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,6 @@ public static UploadStrategy getFromPreferences() {
* @param strategy the strategy to save
*/
public static void saveToPreferences(UploadStrategy strategy) {
Config.getPref().put("osm-server.upload-strategy", strategy.getPreferenceValue());
Config.getPref().put("osm-server.upload-strategy", strategy != null ? strategy.getPreferenceValue() : null);
}
}
100 changes: 100 additions & 0 deletions test/unit/org/openstreetmap/josm/actions/UploadActionTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package org.openstreetmap.josm.actions;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

import java.awt.GraphicsEnvironment;
import java.util.Collections;
import java.util.concurrent.TimeUnit;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.openstreetmap.josm.TestUtils;
import org.openstreetmap.josm.command.AddPrimitivesCommand;
import org.openstreetmap.josm.data.UserIdentityManager;
import org.openstreetmap.josm.data.coor.LatLon;
import org.openstreetmap.josm.data.osm.DataSet;
import org.openstreetmap.josm.data.osm.Node;
import org.openstreetmap.josm.gui.MainApplication;
import org.openstreetmap.josm.gui.io.UploadDialog;
import org.openstreetmap.josm.gui.layer.OsmDataLayer;
import org.openstreetmap.josm.gui.util.GuiHelper;
import org.openstreetmap.josm.testutils.JOSMTestRules;
import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
import org.openstreetmap.josm.testutils.mockers.WindowMocker;
import org.openstreetmap.josm.tools.Logging;

import mockit.Invocation;
import mockit.Mock;
import mockit.MockUp;

/**
* Test class for {@link UploadAction}
* @author Taylor Smock
*/
@BasicPreferences
class UploadActionTest {
// Only needed for layer cleanup. And user identity cleanup. And ensuring that data isn't accidentally uploaded.
@RegisterExtension
JOSMTestRules josmTestRules = new JOSMTestRules().main().projection().fakeAPI();

/**
* Non-regression test for JOSM #21476.
*/
@Test
void testNonRegression21476() {
TestUtils.assumeWorkingJMockit();
Logging.clearLastErrorAndWarnings();
new WindowMocker();
new UploadDialogMock();
// Set up the preconditions. This test acts more like an integration test, in so far as it also tests
// unrelated code.
UserIdentityManager.getInstance().setAnonymous();
final DataSet testData = new DataSet();
MainApplication.getLayerManager().addLayer(new OsmDataLayer(testData, "testNonRegression21476", null));
final Node toAdd = new Node(new LatLon(20, 20));
toAdd.put("shop", "butcher");
final AddPrimitivesCommand command = new AddPrimitivesCommand(Collections.singletonList(toAdd.save()), testData);
command.executeCommand();
final UploadAction uploadAction = new UploadAction();
uploadAction.updateEnabledState();
assertTrue(uploadAction.isEnabled());
// Perform the actual "test" -- we don't want it to throw an exception
assertDoesNotThrow(() -> uploadAction.actionPerformed(null));
// Sync threads
GuiHelper.runInEDT(() -> {/* sync edt */});
try {
MainApplication.worker.submit(() -> {/* sync worker */}).get(1, TimeUnit.SECONDS);
assertTrue(Logging.getLastErrorAndWarnings().isEmpty());
} catch (Exception exception) {
fail(exception);
} finally {
Logging.clearLastErrorAndWarnings();
}
}

private static class UploadDialogMock extends MockUp<UploadDialog> {
@Mock
public void pack(final Invocation invocation) {
if (!GraphicsEnvironment.isHeadless()) {
invocation.proceed();
}
}

@Mock
public void setVisible(final Invocation invocation, final boolean visible) {
if (!GraphicsEnvironment.isHeadless()) {
invocation.proceed(visible);
}
}

@Mock
public final boolean isCanceled(final Invocation invocation) {
if (!GraphicsEnvironment.isHeadless()) {
return invocation.proceed();
}
return true;
}
}
}

0 comments on commit 242640a

Please sign in to comment.