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

7 files changed, 93 insertions, 20 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()) {