[node] Tagging a commit with an existing tag name causes two response…

…s to be sent back

In the tagging code, the promise chain uses a done(*) function for
sending back the response. This causes an issue when an error occurs
such as when the user tries to tag a commit with a name that's
already in use. The catch(*) function will resolve and send an error
response back and then the done(*) function will also resolve and try
to send a response back. The code should instead be structured such
that a success response should only be sent back if everything is
okay. It should not be sending a success response back irregardless
of whether the commit was tagged or not.

Signed-off-by: Remy Suen <remy.suen@gmail.com>

modules/orionode/lib/git/commit.js

@@ -1182,14 +1182,11 @@ function tag(req, res, commitId, name, isAnnotated, message) {
 		return getCommitParents(theRepo, thisCommit, fileDir);
 	})
 	.then(function(parents){
-		theParents = parents;
+		writeResponse(200, res, null, commitJSON(thisCommit, fileDir, theDiffs, parents));
 	})
 	.catch(function(err) {
 		writeError(403, res, err.message);
 	})
-	.done(function() {
-		writeResponse(200, res, null, commitJSON(thisCommit, fileDir, theDiffs, theParents));
-	});
 }
 
 function putCommit(req, res) {

modules/orionode/test/test-git-api.js

@@ -286,7 +286,11 @@ GitClient.prototype = {
 		});
 	},
 
-	createTag: function(commitSHA, tagName, annotated, message) {
+	createTag: function(commitSHA, tagName, annotated, message, expectedStatusCode) {
+		if (expectedStatusCode === undefined) {
+			expectedStatusCode = 200;
+		}
+
 		var client = this;
 		this.tasks.push(function(resolve) {
 			request()
@@ -296,7 +300,7 @@ GitClient.prototype = {
 				Annotated: annotated,
 				Message: message
 			})
-			.expect(200)
+			.expect(expectedStatusCode)
 			.end(function(err, res) {
 				assert.ifError(err);
 				client.next(resolve, res.body);
@@ -3026,6 +3030,48 @@ maybeDescribe("git", function() {
 			it("annotated", function(finished) {
 				testCreateTag(finished, "tag-create-annotated", true);
 			});
+
+			/**
+			 * 1. Create a tag with a name.
+			 * 2. Create a tag on the same commit with the same name.
+			 * 3. Check that the server isn't trying to set headers
+			 *    after a response has been sent.
+			 */
+			it("bug 515315", function(finished) {
+				var testName = "tag-bug515315";
+				var tagName = "tag515315";
+				var commitSHA;
+
+				var client = new GitClient(testName);
+				client.init();
+				client.commit();
+				client.start().then(function(commit) {
+					commitSHA = commit.Id;
+
+					client.createTag(commitSHA, tagName, false, null);
+					client.listTags();
+					return client.start();
+				})
+				.then(function(tags) {
+					// only created one tag
+					assert.equal(tags.length, 1);
+					assertTag(tags[0], tagName, false, testName, commitSHA);
+
+					// create a tag with the same name, this should cause a 403
+					client.createTag(commitSHA, tagName, false, null, 403);
+					client.listTags();
+					return client.start();
+				})
+				.then(function(tags) {
+					// still only one tag
+					assert.equal(tags.length, 1);
+					assertTag(tags[0], tagName, false, testName, commitSHA);
+					finished();
+				})
+				.catch(function(err) {
+					finished(err);
+				});
+			});
 		}); // describe("Create")
 
 		describe("Checkout", function() {