Improved oid normalization

tbranyen · tbranyen · commit e4820100aad2 · 2014-07-11T11:54:36.000-04:00
diff --git a/lib/repository.js b/lib/repository.js
@@ -1,12 +1,13 @@
 var git = require('../');
 var util = require('./util');
 var Refs = require('./reference');
+var normalizeOid = require("./util/normalize_oid");
+
 var Commit = git.Commit;
 var Revwalk = git.Revwalk;
 var Repo = git.Repository;
 var Blob = git.Blob;
 var Tree = git.Tree;
-//var TreeBuilder = git.TreeBuilder;
 var Reference = git.Reference;
 
 // Backwards compatibility.
@@ -79,7 +80,7 @@ util.makeSafe(Repo.prototype, 'getReference');
 Repo.prototype.getCommit = function(oid, callback) {
   var repository = this;
 
-  return Commit.lookup(repository, oid).then(function(commit) {
+  return Commit.lookup(repository, normalizeOid(oid)).then(function(commit) {
     commit.repo = repository;
 
     if (callback) {
@@ -89,8 +90,6 @@ Repo.prototype.getCommit = function(oid, callback) {
     return commit;
   });
 };
-util.normalizeOid(Repo.prototype, 'getCommit');
-util.makeSafe(Repo.prototype, 'getCommit');
 
 /**
  * Retrieve the blob represented by the oid.
@@ -100,9 +99,9 @@ util.makeSafe(Repo.prototype, 'getCommit');
  * @return {Blob}
  */
 Repo.prototype.getBlob = function(oid, callback) {
-  var repository= this;
+  var repository = this;
 
-  return Blob.lookup(repository, oid).then(function(blob) {
+  return Blob.lookup(repository, normalizeOid(oid)).then(function(blob) {
     blob.repo = repository;
 
     if (callback) {
@@ -112,8 +111,6 @@ Repo.prototype.getBlob = function(oid, callback) {
     return blob;
   }, callback);
 };
-util.normalizeOid(Repo.prototype, 'getBlob');
-util.makeSafe(Repo.prototype, 'getBlob');
 
 /**
  * Retrieve the tree represented by the oid.
diff --git a/lib/util.js b/lib/util.js
@@ -4,7 +4,7 @@ exports.makeSafe = function(object, key) {
   var oldFn = object[key];
   object[key] = function() {
     try {
-      return oldFn.apply(this, arguments);
+      oldFn.apply(this, arguments);
     } catch (e) {
       var callback = arguments[arguments.length - 1];
       callback(e);
@@ -16,6 +16,6 @@ exports.normalizeOid = function(object, key) {
   var oldFn = object[key];
   object[key] = function(oid) {
     if (typeof oid === 'string') oid = git.Oid.fromString(oid);
-    return oldFn.apply(this, arguments);
+    oldFn.apply(this, arguments);
   };
 };
diff --git a/lib/util/normalize_oid.js b/lib/util/normalize_oid.js
@@ -0,0 +1,18 @@
+var git = require("../../");
+
+/**
+ * Normalize an identifier to always be an OID instance.
+ *
+ * @param {String, Object} oid - The oid string or instance.
+ * @return {Object} An Oid instance.
+ */
+function normalizeOid(oid) {
+  try {
+    return typeof oid === "string" ? git.Oid.fromString(oid) : oid;
+  }
+  catch (ex) {
+    return null;
+  }
+}
+
+module.exports = normalizeOid;
diff --git a/test/tests/commit.js b/test/tests/commit.js
@@ -24,23 +24,65 @@ describe("Commit", function() {
     });
   });
 
-  it("makes its message available", function() {
+  it("will fail with an invalid sha", function() {
+    return this.repository.getCommit("invalid").then(null, function(error) {
+      assert.ok(error instanceof Error);
+    });
+  });
+
+  it("has a message", function() {
     assert.equal(this.commit.message(), "Update README.md");
   });
 
-  it("makes its sha available", function() {
+  it("has a sha", function() {
     assert.equal(this.commit.sha(), historyCountKnownSHA);
   });
 
-  it("makes its time available", function() {
+  it("has a time", function() {
     assert.equal(this.commit.timeMs(), 1362012884000);
   });
 
-  it("makes its date available", function() {
+  it("has a date", function() {
     assert.equal(this.commit.date().getTime(), 1362012884000);
   });
 
-  it("makes its offset available", function() {
+  it("has an offset", function() {
     assert.equal(this.commit.offset(), 780);
   });
+
+  describe("author", function() {
+    before(function() {
+      this.author = this.commit.author();
+    });
+
+    it("is available", function() {
+      assert.ok(this.author instanceof nodegit.Signature);
+    });
+
+    it("has a name", function() {
+      assert.equal(this.author.name(), "Michael Robinson");
+    });
+
+    it("has an email", function() {
+      assert.equal(this.author.email(), "mike@panmedia.co.nz");
+    });
+  });
+
+  describe("committer", function() {
+    before(function() {
+      this.author = this.commit.committer();
+    });
+
+    it("is available", function() {
+      assert.ok(this.author instanceof nodegit.Signature);
+    });
+
+    it("has a name", function() {
+      assert.equal(this.author.name(), "Michael Robinson");
+    });
+
+    it("has an email", function() {
+      assert.equal(this.author.email(), "mike@panmedia.co.nz");
+    });
+  });
 });
