Index: trunk/src/org/openstreetmap/josm/actions/upload/UploadNotesTask.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/upload/UploadNotesTask.java (revision 7746) +++ trunk/src/org/openstreetmap/josm/actions/upload/UploadNotesTask.java (revision 7748) @@ -45,4 +45,9 @@ Map failedNotes = new HashMap<>(); + /** + * Constructs a new {@code UploadTask}. + * @param title message for the user + * @param monitor progress monitor + */ public UploadTask(String title, ProgressMonitor monitor) { super(title, monitor, false); @@ -71,42 +76,46 @@ Main.debug("found note change to upload"); } - try { - Note newNote; - switch (comment.getNoteAction()) { - case opened: - if (Main.isDebugEnabled()) { - Main.debug("opening new note"); - } - newNote = api.createNote(note.getLatLon(), comment.getText(), monitor); - note.setId(newNote.getId()); - break; - case closed: - if (Main.isDebugEnabled()) { - Main.debug("closing note " + note.getId()); - } - newNote = api.closeNote(note, comment.getText(), monitor); - break; - case commented: - if (Main.isDebugEnabled()) { - Main.debug("adding comment to note " + note.getId()); - } - newNote = api.addCommentToNote(note, comment.getText(), monitor); - break; - case reopened: - if (Main.isDebugEnabled()) { - Main.debug("reopening note " + note.getId()); - } - newNote = api.reopenNote(note, comment.getText(), monitor); - break; - default: - newNote = null; - } - updatedNotes.put(note, newNote); - } catch (Exception e) { - Main.error("Failed to upload note to server: " + note.getId()); - failedNotes.put(note, e); - } + processNoteComment(monitor, api, note, comment); } } + } + } + + private void processNoteComment(ProgressMonitor monitor, OsmApi api, Note note, NoteComment comment) { + try { + Note newNote; + switch (comment.getNoteAction()) { + case opened: + if (Main.isDebugEnabled()) { + Main.debug("opening new note"); + } + newNote = api.createNote(note.getLatLon(), comment.getText(), monitor); + note.setId(newNote.getId()); + break; + case closed: + if (Main.isDebugEnabled()) { + Main.debug("closing note " + note.getId()); + } + newNote = api.closeNote(note, comment.getText(), monitor); + break; + case commented: + if (Main.isDebugEnabled()) { + Main.debug("adding comment to note " + note.getId()); + } + newNote = api.addCommentToNote(note, comment.getText(), monitor); + break; + case reopened: + if (Main.isDebugEnabled()) { + Main.debug("reopening note " + note.getId()); + } + newNote = api.reopenNote(note, comment.getText(), monitor); + break; + default: + newNote = null; + } + updatedNotes.put(note, newNote); + } catch (Exception e) { + Main.error("Failed to upload note to server: " + note.getId()); + failedNotes.put(note, e); } } Index: trunk/src/org/openstreetmap/josm/gui/tagging/ac/AutoCompletionManager.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/tagging/ac/AutoCompletionManager.java (revision 7746) +++ trunk/src/org/openstreetmap/josm/gui/tagging/ac/AutoCompletionManager.java (revision 7748) @@ -60,11 +60,11 @@ */ public static class UserInputTag { - public String key; - public String value; - public boolean defaultKey; + private final String key; + private final String value; + private final boolean defaultKey; /** * Constructor. - * + * * @param key the tag key * @param value the tag value @@ -90,21 +90,11 @@ @Override public boolean equals(Object obj) { - if (obj == null) { + if (obj == null || getClass() != obj.getClass()) { return false; } - if (getClass() != obj.getClass()) { - return false; - } final UserInputTag other = (UserInputTag) obj; - if (!Objects.equals(this.key, other.key)) { - return false; - } - if (!Objects.equals(this.value, other.value)) { - return false; - } - if (this.defaultKey != other.defaultKey) { - return false; - } - return true; + return Objects.equals(this.key, other.key) + && Objects.equals(this.value, other.value) + && this.defaultKey == other.defaultKey; } } @@ -121,14 +111,15 @@ */ protected MultiMap tagCache; - /** - * the same as tagCache but for the preset keys and values - * can be accessed directly - */ - protected static final MultiMap presetTagCache = new MultiMap<>(); + + /** + * the same as tagCache but for the preset keys and values can be accessed directly + */ + protected static final MultiMap PRESET_TAG_CACHE = new MultiMap<>(); + /** * Cache for tags that have been entered by the user. */ - protected static final Set userInputTagCache = new LinkedHashSet<>(); - + protected static final Set USER_INPUT_TAG_CACHE = new LinkedHashSet<>(); + /** * the cached list of member roles @@ -137,13 +128,17 @@ */ protected Set roleCache; - /** - * the same as roleCache but for the preset roles - * can be accessed directly - */ - protected static final Set presetRoleCache = new HashSet<>(); - + + /** + * the same as roleCache but for the preset roles can be accessed directly + */ + protected static final Set PRESET_ROLE_CACHE = new HashSet<>(); + + /** + * Constructs a new {@code AutoCompletionManager}. + * @param ds data set + */ public AutoCompletionManager(DataSet ds) { this.ds = ds; - dirty = true; + this.dirty = true; } @@ -166,5 +161,4 @@ /** * initializes the cache from the primitives in the dataset - * */ protected void rebuild() { @@ -211,4 +205,5 @@ /** * Initialize the cache for presets. This is done only once. + * @param presets Tagging presets to cache */ public static void cachePresets(Collection presets) { @@ -219,5 +214,5 @@ if (ki.key != null && ki.getValues() != null) { try { - presetTagCache.putAll(ki.key, ki.getValues()); + PRESET_TAG_CACHE.putAll(ki.key, ki.getValues()); } catch (NullPointerException e) { Main.error(p+": Unable to cache "+ki); @@ -228,5 +223,5 @@ for (TaggingPresetItems.Role i : r.roles) { if (i.key != null) { - presetRoleCache.add(i.key); + PRESET_ROLE_CACHE.add(i.key); } } @@ -235,10 +230,15 @@ } } - - + + /** + * Remembers user input for the given key/value. + * @param key Tag key + * @param value Tag value + * @param defaultKey true, if the key was not really entered by the user, e.g. for preset text fields + */ public static void rememberUserInput(String key, String value, boolean defaultKey) { UserInputTag tag = new UserInputTag(key, value, defaultKey); - userInputTagCache.remove(tag); // re-add, so it gets to the last position of the LinkedHashSet - userInputTagCache.add(tag); + USER_INPUT_TAG_CACHE.remove(tag); // re-add, so it gets to the last position of the LinkedHashSet + USER_INPUT_TAG_CACHE.add(tag); } @@ -253,10 +253,10 @@ protected List getPresetKeys() { - return new ArrayList<>(presetTagCache.keySet()); - } - + return new ArrayList<>(PRESET_TAG_CACHE.keySet()); + } + protected Collection getUserInputKeys() { List keys = new ArrayList<>(); - for (UserInputTag tag : userInputTagCache) { + for (UserInputTag tag : USER_INPUT_TAG_CACHE) { if (!tag.defaultKey) { keys.add(tag.key); @@ -279,10 +279,10 @@ protected static List getPresetValues(String key) { - return new ArrayList<>(presetTagCache.getValues(key)); + return new ArrayList<>(PRESET_TAG_CACHE.getValues(key)); } protected static Collection getUserInputValues(String key) { - ArrayList values = new ArrayList<>(); - for (UserInputTag tag : userInputTagCache) { + List values = new ArrayList<>(); + for (UserInputTag tag : USER_INPUT_TAG_CACHE) { if (key.equals(tag.key)) { values.add(tag.value); @@ -309,5 +309,5 @@ */ public void populateWithMemberRoles(AutoCompletionList list) { - list.add(presetRoleCache, AutoCompletionItemPriority.IS_IN_STANDARD); + list.add(PRESET_ROLE_CACHE, AutoCompletionItemPriority.IS_IN_STANDARD); list.add(getRoleCache(), AutoCompletionItemPriority.IS_IN_DATASET); } Index: trunk/src/org/openstreetmap/josm/io/OsmApi.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/OsmApi.java (revision 7746) +++ trunk/src/org/openstreetmap/josm/io/OsmApi.java (revision 7748) @@ -332,5 +332,8 @@ // this works around a ruby (or lighttpd) bug where two consecutive slashes in // an URL will cause a "404 not found" response. - int p; while ((p = rv.indexOf("//", rv.indexOf("://")+2)) > -1) { rv.delete(p, p + 1); } + int p; + while ((p = rv.indexOf("//", rv.indexOf("://")+2)) > -1) { + rv.delete(p, p + 1); + } return rv.toString(); } @@ -383,5 +386,6 @@ osm.setVisible(true); } catch(NumberFormatException e) { - throw new OsmTransferException(tr("Unexpected format of new version of modified primitive ''{0}''. Got ''{1}''.", osm.getId(), ret)); + throw new OsmTransferException(tr("Unexpected format of new version of modified primitive ''{0}''. Got ''{1}''.", + osm.getId(), ret)); } } @@ -467,6 +471,7 @@ throw e; } catch(OsmApiException e) { - if (e.getResponseCode() == HttpURLConnection.HTTP_CONFLICT && ChangesetClosedException.errorHeaderMatchesPattern(e.getErrorHeader())) - throw new ChangesetClosedException(e.getErrorHeader(), ChangesetClosedException.Source.UPDATE_CHANGESET); + String errorHeader = e.getErrorHeader(); + if (e.getResponseCode() == HttpURLConnection.HTTP_CONFLICT && ChangesetClosedException.errorHeaderMatchesPattern(errorHeader)) + throw new ChangesetClosedException(errorHeader, ChangesetClosedException.Source.UPDATE_CHANGESET); throw e; } finally { @@ -512,5 +517,6 @@ * @throws OsmTransferException if something is wrong */ - public Collection uploadDiff(Collection list, ProgressMonitor monitor) throws OsmTransferException { + public Collection uploadDiff(Collection list, ProgressMonitor monitor) + throws OsmTransferException { try { monitor.beginTask("", list.size() * 2); @@ -588,5 +594,6 @@ } - protected final String sendRequest(String requestMethod, String urlSuffix,String requestBody, ProgressMonitor monitor) throws OsmTransferException { + protected final String sendRequest(String requestMethod, String urlSuffix,String requestBody, ProgressMonitor monitor) + throws OsmTransferException { return sendRequest(requestMethod, urlSuffix, requestBody, monitor, true, false); } @@ -611,5 +618,6 @@ * been exhausted), or rewrapping a Java exception. */ - protected final String sendRequest(String requestMethod, String urlSuffix,String requestBody, ProgressMonitor monitor, boolean doAuthenticate, boolean fastFail) throws OsmTransferException { + protected final String sendRequest(String requestMethod, String urlSuffix,String requestBody, ProgressMonitor monitor, + boolean doAuthenticate, boolean fastFail) throws OsmTransferException { StringBuilder responseBody = new StringBuilder(); int retries = fastFail ? 0 : getMaxRetries(); @@ -788,6 +796,10 @@ } - /** - * Create a new note on the server + private static StringBuilder noteStringBuilder(Note note) { + return new StringBuilder().append("notes/").append(note.getId()); + } + + /** + * Create a new note on the server. * @param latlon Location of note * @param text Comment entered by user to open the note @@ -798,5 +810,5 @@ public Note createNote(LatLon latlon, String text, ProgressMonitor monitor) throws OsmTransferException { initialize(monitor); - String url = new StringBuilder() + String noteUrl = new StringBuilder() .append("notes?lat=") .append(latlon.lat()) @@ -806,5 +818,5 @@ .append(urlEncode(text)).toString(); - String response = sendRequest("POST", url, null, monitor, true, false); + String response = sendRequest("POST", noteUrl, null, monitor, true, false); return parseSingleNote(response); } @@ -820,16 +832,14 @@ public Note addCommentToNote(Note note, String comment, ProgressMonitor monitor) throws OsmTransferException { initialize(monitor); - String url = new StringBuilder() - .append("notes/") - .append(note.getId()) + String noteUrl = noteStringBuilder(note) .append("/comment?text=") .append(urlEncode(comment)).toString(); - String response = sendRequest("POST", url, null, monitor, true, false); + String response = sendRequest("POST", noteUrl, null, monitor, true, false); return parseSingleNote(response); } /** - * Close a note + * Close a note. * @param note Note to close. Must currently be open * @param closeMessage Optional message supplied by the user when closing the note @@ -841,7 +851,5 @@ initialize(monitor); String encodedMessage = urlEncode(closeMessage); - StringBuilder urlBuilder = new StringBuilder() - .append("notes/") - .append(note.getId()) + StringBuilder urlBuilder = noteStringBuilder(note) .append("/close"); if (encodedMessage != null && !encodedMessage.trim().isEmpty()) { @@ -865,7 +873,5 @@ initialize(monitor); String encodedMessage = urlEncode(reactivateMessage); - StringBuilder urlBuilder = new StringBuilder() - .append("notes/") - .append(note.getId()) + StringBuilder urlBuilder = noteStringBuilder(note) .append("/reopen"); if (encodedMessage != null && !encodedMessage.trim().isEmpty()) { Index: trunk/src/org/openstreetmap/josm/tools/ImageProvider.java =================================================================== --- trunk/src/org/openstreetmap/josm/tools/ImageProvider.java (revision 7746) +++ trunk/src/org/openstreetmap/josm/tools/ImageProvider.java (revision 7748) @@ -91,4 +91,8 @@ public class ImageProvider { + private static final String HTTP_PROTOCOL = "http://"; + private static final String HTTPS_PROTOCOL = "https://"; + private static final String WIKI_PROTOCOL = "wiki://"; + /** * Position of an overlay icon @@ -266,5 +270,5 @@ * @since 7687 */ - static public Dimension getImageSizes(ImageSizes size) { + public static Dimension getImageSizes(ImageSizes size) { int sizeval; switch(size) { @@ -458,5 +462,5 @@ */ public void getInBackground(final ImageCallback callback) { - if (name.startsWith("http://") || name.startsWith("wiki://")) { + if (name.startsWith(HTTP_PROTOCOL) || name.startsWith(WIKI_PROTOCOL)) { Runnable fetch = new Runnable() { @Override @@ -486,5 +490,5 @@ */ public void getInBackground(final ImageResourceCallback callback) { - if (name.startsWith("http://") || name.startsWith("wiki://")) { + if (name.startsWith(HTTP_PROTOCOL) || name.startsWith(WIKI_PROTOCOL)) { Runnable fetch = new Runnable() { @Override @@ -572,5 +576,5 @@ ImageType type = name.toLowerCase().endsWith(".svg") ? ImageType.SVG : ImageType.OTHER; - if (name.startsWith("http://") || name.startsWith("https://")) { + if (name.startsWith(HTTP_PROTOCOL) || name.startsWith(HTTPS_PROTOCOL)) { String url = name; ImageResource ir = cache.get(url); @@ -581,5 +585,5 @@ } return ir; - } else if (name.startsWith("wiki://")) { + } else if (name.startsWith(WIKI_PROTOCOL)) { ImageResource ir = cache.get(name); if (ir != null) return ir; @@ -984,4 +988,5 @@ * overlay must be transparent in the background. * Also scaling is not cared about with current implementation. + * @deprecated this method will be refactored */ @Deprecated