Bug 517621 - Stash REST API gives ambiguous errors if the stash is empty

If the stash is empty and a client tries to contact the Orion server
to drop the entire stash or a given stash revision, the server should
send back an error message that indicates that the stash is empty.

If the stash is not empty and a client tries to drop the stash with
an invalid stash revision, the server should send back an error
message to notify the user that the provided stash revision is
invalid and that the specified stash revision could not be found in
the repository's list of refs.

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

modules/orionode/lib/git/stash.js

@@ -113,8 +113,10 @@ function deleteStash(req, res) {
 	return clone.getRepo(req)
 	.then(function(_repo) {
 		repo = _repo;
-		var index, all = [];
+		var index = -1, all = [];
+		var empty = true;
 		return git.Stash.foreach(repo, /* @callback */ function(i, message, oid) {
+			empty = false;
 			if (stashRev) {
 				if (oid.toString() === stashRev) {
 					index = i;
@@ -124,12 +126,29 @@ function deleteStash(req, res) {
 			} 
 		})
 		.then(function() {
-			if (all.length) return Promise.all(all);
-			return git.Stash.drop(repo, index);
+			if (empty) {
+				return "Failed to drop stashed changes due to an empty stash.";
+			} else if (stashRev) {
+				if (index === -1) {
+					return "Invalid stash reference " + stashRev + ".";
+				} else {
+					return git.Stash.drop(repo, index).then(function() {
+						return null;
+					});
+				}
+			} else {
+				return Promise.all(all).then(function() {
+					return null;
+				});
+			}
 		});
 	})
-	.then(function() {
-		res.status(200).end();
+	.then(function(message) {
+		if (message === null) {
+			res.status(200).end();
+		} else {
+			writeError(400, res, message);
+		}
 	})
 	.catch(function(err) {
 		writeError(404, res, err.message);

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

@@ -484,6 +484,26 @@ GitClient.prototype = {
 		});
 	},
 
+	stashDrop: function(revision, statusCode, message) {
+		if (typeof statusCode !== 'number') {
+			statusCode = 200;
+		}
+
+		var client = this;
+		this.tasks.push(function(resolve) {
+			request()
+			.delete(CONTEXT_PATH + "/gitapi/stash/" + revision + FILE_ROOT + client.getName())
+			.expect(statusCode)
+			.end(function(err, res) {
+				assert.ifError(err);
+				if (statusCode !== 200) {
+					assert.equal(res.body.Message, message);
+				}
+				client.next(resolve, res.body);
+			});
+		});
+	},
+
 	reset: function(type, id) {
 		var client = this;
 		this.tasks.push(function(resolve) {
@@ -3843,6 +3863,67 @@ maybeDescribe("git", function() {
 				});
 			}); // it("simple")
 		}); // describe("Pop")
+
+		describe("Drop", function() {
+
+			/**
+			 * Drop the stash when it is empty.
+			 */
+			it("empty stash", function(finished) {
+				var client = new GitClient("stash-drop-empty");
+				// init a new Git repository
+				client.init();
+				// try dropping the whole stash
+				client.stashDrop(null, 400, "Failed to drop stashed changes due to an empty stash.");
+				client.start().then(function(body) {
+					finished();
+				})
+				.catch(function(err) {
+					finished(err);
+				});
+			}); // it("empty stash")"
+
+			/**
+			 * Drop a given stash revision when the stash is empty.
+			 */
+			it("empty stash with stash rev", function(finished) {
+				var client = new GitClient("stash-drop-empty-stash-rev");
+				// init a new Git repository
+				client.init();
+				// try dropping a stash revision while the stash is not empty
+				client.stashDrop("rev123", 400, "Failed to drop stashed changes due to an empty stash.");
+				client.start().then(function(body) {
+					finished();
+				})
+				.catch(function(err) {
+					finished(err);
+				});
+			}); // it("empty stash with invalid stash rev")"
+
+			/**
+			 * Put something in the stash and then ask Orion to drop an invalid stash revision from the stash.
+			 */
+			it("invalid stash rev", function(finished) {
+				var file = "a.txt";
+				var client = new GitClient("stash-drop-invalid");
+				client.init();
+				// track this file
+				client.setFileContents(file, "abc");
+				client.stage(file);
+				client.commit();
+
+				// modify the file
+				client.setFileContents(file, "abcx");
+				client.stash();
+				client.stashDrop("rev123", 400, "Invalid stash reference rev123.");
+				client.start().then(function(body) {
+					finished();
+				})
+				.catch(function(err) {
+					finished(err);
+				});
+			}); // it("invalid stash rev")"
+		}); // describea("Drop")
 	}); // describe("Stash")
 
 	describe("config", function() {