diff options
author | spingel | 2010-07-09 21:05:05 +0000 |
---|---|---|
committer | spingel | 2010-07-09 21:05:05 +0000 |
commit | 75e8bd19e37ab23b1838c3fb42ae17be8dd4b812 (patch) | |
tree | 7dd3659a90cbb14bf8d2a534a9bd97134799125b | |
parent | 42bd44707ef76bc2e53bbfdf69c62a74cb3ce0a0 (diff) | |
download | org.eclipse.mylyn.tasks-75e8bd19e37ab23b1838c3fb42ae17be8dd4b812.tar.gz org.eclipse.mylyn.tasks-75e8bd19e37ab23b1838c3fb42ae17be8dd4b812.tar.xz org.eclipse.mylyn.tasks-75e8bd19e37ab23b1838c3fb42ae17be8dd4b812.zip |
NEW - bug 253932: [upstream] submitting changes also sends unchanged fields possibly resetting them to old values
https://bugs.eclipse.org/bugs/show_bug.cgi?id=253932
9 files changed, 125 insertions, 22 deletions
diff --git a/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/TracAttribute.java b/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/TracAttribute.java index 56635e690..5c69746ad 100644 --- a/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/TracAttribute.java +++ b/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/TracAttribute.java @@ -26,26 +26,33 @@ public enum TracAttribute { CC(Key.CC, Messages.TracAttribute_CC, TaskAttribute.USER_CC, TaskAttribute.TYPE_SHORT_TEXT, Flag.PEOPLE), - CHANGE_TIME(Key.CHANGE_TIME, Messages.TracAttribute_Last_Modification, TaskAttribute.DATE_MODIFICATION, TaskAttribute.TYPE_DATE, - Flag.READ_ONLY), + CHANGE_TIME(Key.CHANGE_TIME, Messages.TracAttribute_Last_Modification, TaskAttribute.DATE_MODIFICATION, + TaskAttribute.TYPE_DATE, Flag.READ_ONLY), - COMPONENT(Key.COMPONENT, Messages.TracAttribute_Component, TaskAttribute.PRODUCT, TaskAttribute.TYPE_SINGLE_SELECT, Flag.ATTRIBUTE), + COMPONENT(Key.COMPONENT, Messages.TracAttribute_Component, TaskAttribute.PRODUCT, TaskAttribute.TYPE_SINGLE_SELECT, + Flag.ATTRIBUTE), - DESCRIPTION(Key.DESCRIPTION, Messages.TracAttribute_Description, TaskAttribute.DESCRIPTION, TaskAttribute.TYPE_LONG_RICH_TEXT), + DESCRIPTION(Key.DESCRIPTION, Messages.TracAttribute_Description, TaskAttribute.DESCRIPTION, + TaskAttribute.TYPE_LONG_RICH_TEXT), ID(Key.ID, Messages.TracAttribute_ID, TaskAttribute.TASK_KEY, TaskAttribute.TYPE_SHORT_TEXT, Flag.PEOPLE), - KEYWORDS(Key.KEYWORDS, Messages.TracAttribute_Keywords, TaskAttribute.KEYWORDS, TaskAttribute.TYPE_SHORT_TEXT, Flag.ATTRIBUTE), + KEYWORDS(Key.KEYWORDS, Messages.TracAttribute_Keywords, TaskAttribute.KEYWORDS, TaskAttribute.TYPE_SHORT_TEXT, + Flag.ATTRIBUTE), MILESTONE(Key.MILESTONE, Messages.TracAttribute_Milestone, null, TaskAttribute.TYPE_SINGLE_SELECT, Flag.ATTRIBUTE), - OWNER(Key.OWNER, Messages.TracAttribute_Assigned_to, TaskAttribute.USER_ASSIGNED, TaskAttribute.TYPE_PERSON, Flag.PEOPLE), + OWNER(Key.OWNER, Messages.TracAttribute_Assigned_to, TaskAttribute.USER_ASSIGNED, TaskAttribute.TYPE_PERSON, + Flag.PEOPLE), - PRIORITY(Key.PRIORITY, Messages.TracAttribute_Priority, TaskAttribute.PRIORITY, TaskAttribute.TYPE_SINGLE_SELECT, Flag.ATTRIBUTE), + PRIORITY(Key.PRIORITY, Messages.TracAttribute_Priority, TaskAttribute.PRIORITY, TaskAttribute.TYPE_SINGLE_SELECT, + Flag.ATTRIBUTE), - REPORTER(Key.REPORTER, Messages.TracAttribute_Reporter, TaskAttribute.USER_REPORTER, TaskAttribute.TYPE_PERSON, Flag.READ_ONLY), + REPORTER(Key.REPORTER, Messages.TracAttribute_Reporter, TaskAttribute.USER_REPORTER, TaskAttribute.TYPE_PERSON, + Flag.READ_ONLY), - RESOLUTION(Key.RESOLUTION, Messages.TracAttribute_Resolution, TaskAttribute.RESOLUTION, TaskAttribute.TYPE_SINGLE_SELECT), + RESOLUTION(Key.RESOLUTION, Messages.TracAttribute_Resolution, TaskAttribute.RESOLUTION, + TaskAttribute.TYPE_SINGLE_SELECT), SEVERITY(Key.SEVERITY, Messages.TracAttribute_Severity, null, TaskAttribute.TYPE_SINGLE_SELECT, Flag.ATTRIBUTE), @@ -55,9 +62,12 @@ public enum TracAttribute { TIME(Key.TIME, Messages.TracAttribute_Created, TaskAttribute.DATE_CREATION, TaskAttribute.TYPE_DATE, Flag.READ_ONLY), - TYPE(Key.TYPE, Messages.TracAttribute_Type, TaskAttribute.TASK_KIND, TaskAttribute.TYPE_SINGLE_SELECT, Flag.ATTRIBUTE), + TYPE(Key.TYPE, Messages.TracAttribute_Type, TaskAttribute.TASK_KIND, TaskAttribute.TYPE_SINGLE_SELECT, + Flag.ATTRIBUTE), + + VERSION(Key.VERSION, Messages.TracAttribute_Version, null, TaskAttribute.TYPE_SINGLE_SELECT, Flag.ATTRIBUTE), - VERSION(Key.VERSION, Messages.TracAttribute_Version, null, TaskAttribute.TYPE_SINGLE_SELECT, Flag.ATTRIBUTE); + TOKEN(Key.TOKEN, "Update Token", null, TaskAttribute.TYPE_SHORT_TEXT, Flag.READ_ONLY); //$NON-NLS-1$ static Map<String, TracAttribute> attributeByTracKey = new HashMap<String, TracAttribute>(); diff --git a/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/TracCorePlugin.java b/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/TracCorePlugin.java index da14be4f5..7e1fc65cf 100644 --- a/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/TracCorePlugin.java +++ b/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/TracCorePlugin.java @@ -20,6 +20,7 @@ import org.eclipse.core.runtime.Plugin; import org.eclipse.mylyn.internal.trac.core.client.InvalidTicketException; import org.eclipse.mylyn.internal.trac.core.client.TracException; import org.eclipse.mylyn.internal.trac.core.client.TracLoginException; +import org.eclipse.mylyn.internal.trac.core.client.TracMidAirCollisionException; import org.eclipse.mylyn.internal.trac.core.client.TracPermissionDeniedException; import org.eclipse.mylyn.internal.trac.core.util.TracUtil; import org.eclipse.mylyn.tasks.core.RepositoryStatus; @@ -92,6 +93,8 @@ public class TracCorePlugin extends Plugin { } else if (e instanceof InvalidTicketException) { return new RepositoryStatus(repository.getRepositoryUrl(), IStatus.ERROR, ID_PLUGIN, RepositoryStatus.ERROR_IO, Messages.TracCorePlugin_the_SERVER_RETURNED_an_UNEXPECTED_RESOPNSE, e); + } else if (e instanceof TracMidAirCollisionException) { + return RepositoryStatus.createCollisionError(repository.getUrl(), TracCorePlugin.ID_PLUGIN); } else if (e instanceof TracException) { String message = e.getMessage(); if (message == null) { @@ -106,8 +109,8 @@ public class TracCorePlugin extends Plugin { return new RepositoryStatus(IStatus.ERROR, ID_PLUGIN, RepositoryStatus.ERROR_IO, Messages.TracCorePlugin_Repository_URL_is_invalid, e); } else { - return new RepositoryStatus(IStatus.ERROR, ID_PLUGIN, RepositoryStatus.ERROR_INTERNAL, Messages.TracCorePlugin_Unexpected_error, - e); + return new RepositoryStatus(IStatus.ERROR, ID_PLUGIN, RepositoryStatus.ERROR_INTERNAL, + Messages.TracCorePlugin_Unexpected_error, e); } } diff --git a/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/TracTaskDataHandler.java b/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/TracTaskDataHandler.java index eee7fa9fd..650c77964 100644 --- a/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/TracTaskDataHandler.java +++ b/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/TracTaskDataHandler.java @@ -298,6 +298,8 @@ public class TracTaskDataHandler extends AbstractTaskDataHandler { TaskAttribute attribute = createAttribute(data, client, TracAttribute.RESOLUTION); // reset default value to avoid "fixed" resolution on tasks created through web attribute.setValue(""); //$NON-NLS-1$ + // internal attributes + createAttribute(data, null, TracAttribute.TOKEN); } createAttribute(data, client, TracAttribute.COMPONENT); createAttribute(data, client, TracAttribute.VERSION); @@ -566,7 +568,7 @@ public class TracTaskDataHandler extends AbstractTaskDataHandler { if (TracAttributeMapper.isInternalAttribute(attribute) || TracAttribute.RESOLUTION.getTracKey().equals(attribute.getId())) { // ignore internal attributes, resolution is set through operations - } else if (!attribute.getMetaData().isReadOnly()) { + } else if (!attribute.getMetaData().isReadOnly() || Key.TOKEN.getKey().equals(attribute.getId())) { ticket.putValue(attribute.getId(), attribute.getValue()); } } @@ -620,6 +622,10 @@ public class TracTaskDataHandler extends AbstractTaskDataHandler { ticket.putValue("action", action); //$NON-NLS-1$ } + Date lastChanged = TracUtil.parseDate(TracRepositoryConnector.getAttributeValue(data, + TracAttribute.CHANGE_TIME.getTracKey())); + ticket.setLastChanged(lastChanged); + return ticket; } diff --git a/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/client/InvalidWikiPageException.java b/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/client/InvalidWikiPageException.java index 88fac57d2..760525f7e 100644 --- a/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/client/InvalidWikiPageException.java +++ b/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/client/InvalidWikiPageException.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2006, 2008 Steffen Pingel and others. + * Copyright (c) 2006, 2008 Xiaoyang Guan and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at diff --git a/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/client/TracMidAirCollisionException.java b/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/client/TracMidAirCollisionException.java new file mode 100644 index 000000000..c16ecef8d --- /dev/null +++ b/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/client/TracMidAirCollisionException.java @@ -0,0 +1,38 @@ +/******************************************************************************* + * Copyright (c) 2010 Tasktop Technologies and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Tasktop Technologies - initial API and implementation + *******************************************************************************/ + +package org.eclipse.mylyn.internal.trac.core.client; + +/** + * Indicates that an edit conflict has occurred due to a ticket changing in the repository since it was last updated. + * + * @author Steffen Pingel + */ +public class TracMidAirCollisionException extends TracRemoteException { + + private static final long serialVersionUID = -5505542112062812372L; + + public TracMidAirCollisionException() { + } + + public TracMidAirCollisionException(String message) { + super(message); + } + + public TracMidAirCollisionException(Throwable cause) { + super(cause); + } + + public TracMidAirCollisionException(String message, Throwable cause) { + super(message, cause); + } + +} diff --git a/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/client/TracXmlRpcClient.java b/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/client/TracXmlRpcClient.java index 45e0110e5..7bcae7f8c 100644 --- a/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/client/TracXmlRpcClient.java +++ b/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/client/TracXmlRpcClient.java @@ -94,7 +94,11 @@ import org.eclipse.osgi.util.NLS; */ public class TracXmlRpcClient extends AbstractTracClient implements ITracWikiClient { - private static final Pattern RPC_METHOD_NOT_FOUND_PATTERN = Pattern.compile("RPC method \".*\" not found"); //$NON-NLS-1$ + private static final Pattern ERROR_PATTERN_RPC_METHOD_NOT_FOUND = Pattern.compile("RPC method \".*\" not found"); //$NON-NLS-1$ + + private static final Pattern ERROR_PATTERN_MID_AIR_COLLISION = Pattern.compile("Sorry, can not save your changes.*This ticket has been modified by someone else since you started"); //$NON-NLS-1$ + + private static final String ERROR_XML_RPC_PRIVILEGES_REQUIRED = "XML_RPC privileges are required to perform this operation"; //$NON-NLS-1$ private class XmlRpcRequest { @@ -169,13 +173,14 @@ public class TracXmlRpcClient extends AbstractTracClient implements ITracWikiCli throw new TracException(e); } catch (XmlRpcException e) { // XXX work-around for http://trac-hacks.org/ticket/5848 - if ("XML_RPC privileges are required to perform this operation".equals(e.getMessage()) //$NON-NLS-1$ - || e.code == XML_FAULT_PERMISSION_DENIED) { + if (ERROR_XML_RPC_PRIVILEGES_REQUIRED.equals(e.getMessage()) || e.code == XML_FAULT_PERMISSION_DENIED) { handleAuthenticationException(HttpStatus.SC_FORBIDDEN, null); // should never happen as call above should always throw an exception throw new TracRemoteException(e); } else if (isNoSuchMethodException(e)) { throw new TracNoSuchMethodException(e); + } else if (isMidAirCollision(e)) { + throw new TracMidAirCollisionException(e); } else { throw new TracRemoteException(e); } @@ -186,6 +191,14 @@ public class TracXmlRpcClient extends AbstractTracClient implements ITracWikiCli } } + private boolean isMidAirCollision(XmlRpcException e) { + if (e.code == XML_FAULT_GENERAL_ERROR && e.getMessage() != null + && ERROR_PATTERN_MID_AIR_COLLISION.matcher(e.getMessage()).find()) { + return true; + } + return false; + } + private boolean isNoSuchMethodException(XmlRpcException e) { // the fault code is used for various errors, therefore detection is based on the message // message format by XML-RPC Plugin version: @@ -193,7 +206,7 @@ public class TracXmlRpcClient extends AbstractTracClient implements ITracWikiCli // 1.0.6: RPC method "ticket.ge1t" not found // 1.10: RPC method "ticket.ge1t" not found' while executing 'ticket.ge1t() if (e.code == XML_FAULT_GENERAL_ERROR && e.getMessage() != null - && RPC_METHOD_NOT_FOUND_PATTERN.matcher(e.getMessage()).find()) { + && ERROR_PATTERN_RPC_METHOD_NOT_FOUND.matcher(e.getMessage()).find()) { return true; } return false; @@ -955,7 +968,10 @@ public class TracXmlRpcClient extends AbstractTracClient implements ITracWikiCli if (!supportsWorkFlow(monitor)) { // submitted as part of status and resolution updates for Trac < 0.11 attributes.remove("action"); //$NON-NLS-1$ + // avoid confusing older XML-RPC plugin versions + attributes.remove(Key.TOKEN.getKey()); } + if (supportsNotifications(monitor)) { call(monitor, "ticket.update", ticket.getId(), comment, attributes, true); //$NON-NLS-1$ } else { diff --git a/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/model/TracTicket.java b/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/model/TracTicket.java index 4fcb42878..e08b24180 100644 --- a/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/model/TracTicket.java +++ b/org.eclipse.mylyn.trac.core/src/org/eclipse/mylyn/internal/trac/core/model/TracTicket.java @@ -35,7 +35,7 @@ public class TracTicket { CC("cc"), CHANGE_TIME("changetime"), COMPONENT("component"), DESCRIPTION("description"), ID("id"), KEYWORDS( //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ "keywords"), MILESTONE("milestone"), OWNER("owner"), PRIORITY("priority"), REPORTER("reporter"), RESOLUTION( //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ "resolution"), STATUS("status"), SEVERITY("severity"), SUMMARY("summary"), TIME("time"), TYPE("type"), VERSION( //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ //$NON-NLS-6$ - "version"); //$NON-NLS-1$ + "version"), TOKEN("_ts"); //$NON-NLS-1$ //$NON-NLS-2$ public static Key fromKey(String name) { for (Key key : Key.values()) { diff --git a/org.eclipse.mylyn.trac.tests/src/org/eclipse/mylyn/trac/tests/core/TracTaskDataHandlerXmlRpcTest.java b/org.eclipse.mylyn.trac.tests/src/org/eclipse/mylyn/trac/tests/core/TracTaskDataHandlerXmlRpcTest.java index 1d5d6aa0a..0303b66c7 100644 --- a/org.eclipse.mylyn.trac.tests/src/org/eclipse/mylyn/trac/tests/core/TracTaskDataHandlerXmlRpcTest.java +++ b/org.eclipse.mylyn.trac.tests/src/org/eclipse/mylyn/trac/tests/core/TracTaskDataHandlerXmlRpcTest.java @@ -377,8 +377,9 @@ public class TracTaskDataHandlerXmlRpcTest extends TestCase { operation = taskData.getAttributeMapper().getTaskOperation(operations.get(2)); assertEquals("resolve", operation.getOperationId()); assertNotNull(operation.getLabel()); - String associatedId = operation.getTaskAttribute().getMetaData().getValue( - TaskAttribute.META_ASSOCIATED_ATTRIBUTE_ID); + String associatedId = operation.getTaskAttribute() + .getMetaData() + .getValue(TaskAttribute.META_ASSOCIATED_ATTRIBUTE_ID); assertNotNull(associatedId); if (hasReassign) { @@ -409,4 +410,30 @@ public class TracTaskDataHandlerXmlRpcTest extends TestCase { assertEquals("", attribute.getValue()); } + public void testPostTaskDataMidAirCollision() throws Exception { + TracTicket ticket = TracTestUtil.createTicket(client, "midAirCollision"); + if (ticket.getValue(Key.TOKEN) == null) { + // repository does not have mid-air collision support + System.err.println("Skipping TracTaskDataHandler.testPostTaskDataMidAirCollision() due to lack of mid-air collision support on " + + repository.getRepositoryUrl()); + return; + } + TaskData taskData = taskDataHandler.getTaskData(repository, ticket.getId() + "", new NullProgressMonitor()); + TaskAttribute attribute = taskData.getRoot().getMappedAttribute(TaskAttribute.PRIORITY); + attribute.setValue("blocker"); + + // change ticket in repository + ticket.putBuiltinValue(Key.PRIORITY, "trivial"); + client.updateTicket(ticket, "changing priority", null); + + // submit conflicting change + try { + taskDataHandler.postTaskData(repository, taskData, null, new NullProgressMonitor()); + fail("Expected CoreException due to mid-air collision"); + } catch (CoreException e) { + assertEquals(RepositoryStatus.createCollisionError(repository.getRepositoryUrl(), TracCorePlugin.ID_PLUGIN) + .getMessage(), e.getStatus().getMessage()); + } + } + } diff --git a/org.eclipse.mylyn.trac.tests/src/org/eclipse/mylyn/trac/tests/support/TracTestUtil.java b/org.eclipse.mylyn.trac.tests/src/org/eclipse/mylyn/trac/tests/support/TracTestUtil.java index b42dcd4e0..7262d06da 100644 --- a/org.eclipse.mylyn.trac.tests/src/org/eclipse/mylyn/trac/tests/support/TracTestUtil.java +++ b/org.eclipse.mylyn.trac.tests/src/org/eclipse/mylyn/trac/tests/support/TracTestUtil.java @@ -105,6 +105,9 @@ public class TracTestUtil { expected = expectedString.substring(0, i + 1) + "\u2026"; } } + } else if (key.startsWith("_")) { + // ignore internal values + continue; } Assert.assertEquals("Values for key '" + key + "' did not match", expected, actual); } |