Merge pull request #933 from callmehiphop/vm-detach-disk-bug

compute: detachDisk disk cache
googleapis · Nov 9, 2015 · eaef49d · eaef49d
2 parents 638c876 + 79cc29b
commit eaef49d
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 66 deletions.
diff --git a/lib/compute/vm.js b/lib/compute/vm.js
@@ -192,45 +192,38 @@ VM.prototype.detachDisk = function(disk, callback) {
     throw new Error('A Disk object must be provided.');
   }
 
-  var disks = this.metadata.disks;
-
-  if (is.empty(disks)) {
-    this.getMetadata(function(err) {
-      if (err) {
-        callback(new DetachDiskError(err.message));
-        return;
-      }
-
-      self.detachDisk(disk, callback);
-    });
-
-    return;
-  }
+  this.getMetadata(function(err, metadata) {
+    if (err) {
+      callback(new DetachDiskError(err.message));
+      return;
+    }
 
-  var deviceName;
-  var baseUrl = 'https://www.googleapis.com/compute/v1/';
+    var deviceName;
+    var baseUrl = 'https://www.googleapis.com/compute/v1/';
+    var disks = metadata.disks || [];
 
-  // Try to find the deviceName by matching the source of the attached disks to
-  // the name of the disk provided by the user.
-  for (var i = 0; !deviceName && i < disks.length; i++) {
-    var attachedDisk = disks[i];
-    var source = attachedDisk.source.replace(baseUrl, '');
+    // Try to find the deviceName by matching the source of the attached disks
+    // to the name of the disk provided by the user.
+    for (var i = 0; !deviceName && i < disks.length; i++) {
+      var attachedDisk = disks[i];
+      var source = attachedDisk.source.replace(baseUrl, '');
 
-    if (source === disk.formattedName) {
-      deviceName = attachedDisk.deviceName;
+      if (source === disk.formattedName) {
+        deviceName = attachedDisk.deviceName;
+      }
     }
-  }
 
-  if (!deviceName) {
-    callback(new DetachDiskError('A device name for this disk was not found.'));
-    return;
-  }
+    if (!deviceName) {
+      callback(new DetachDiskError('Device name for this disk was not found.'));
+      return;
+    }
 
-  var query = {
-    deviceName: deviceName
-  };
+    var query = {
+      deviceName: deviceName
+    };
 
-  this.makeReq_('POST', '/detachDisk', query, null, callback || util.noop);
+    self.makeReq_('POST', '/detachDisk', query, null, callback || util.noop);
+  });
 };
 
 /**

diff --git a/test/compute/vm.js b/test/compute/vm.js
@@ -141,14 +141,18 @@ describe('VM', function() {
       name: DISK.formattedName
     });
 
+    var METADATA = {
+      disks: [
+        {
+          source: DEVICE_NAME,
+          deviceName: DEVICE_NAME
+        }
+      ]
+    };
+
     beforeEach(function() {
-      vm.metadata = {
-        disks: [
-          {
-            source: DEVICE_NAME,
-            deviceName: DEVICE_NAME
-          }
-        ]
+      vm.getMetadata = function(callback) {
+        callback(null, METADATA, METADATA);
       };
     });
 
@@ -164,7 +168,7 @@ describe('VM', function() {
     });
 
     it('should return an error if device name not found', function(done) {
-      vm.metadata = {
+      var metadata = {
         disks: [
           {
             source: 'a',
@@ -173,10 +177,14 @@ describe('VM', function() {
         ]
       };
 
+      vm.getMetadata = function(callback) {
+        callback(null, metadata, metadata);
+      };
+
       vm.detachDisk(DISK, function(err) {
         assert.strictEqual(err.name, 'DetachDiskError');
 
-        var errorMessage = 'A device name for this disk was not found.';
+        var errorMessage = 'Device name for this disk was not found.';
         assert.strictEqual(err.message, errorMessage);
 
         done();
@@ -208,10 +216,6 @@ describe('VM', function() {
     });
 
     describe('refreshing metadata', function() {
-      beforeEach(function() {
-        vm.metadata = {};
-      });
-
       describe('error', function() {
         var ERROR = new Error('Error.');
 
@@ -229,27 +233,6 @@ describe('VM', function() {
           });
         });
       });
-
-      describe('success', function() {
-        beforeEach(function() {
-          vm.getMetadata = function(callback) {
-            callback();
-          };
-        });
-
-        it('should call detachDisk again', function(done) {
-          vm.getMetadata = function(callback) {
-            vm.detachDisk = function(disk, callback) {
-              assert.strictEqual(disk, DISK);
-              callback(); // done()
-            };
-
-            callback();
-          };
-
-          vm.detachDisk(DISK, done);
-        });
-      });
     });
   });