Correct updating baremetal nodes by name or ID

Currently any resource created via Resource.new is marked as clean from
the point of view of JSON patch. This is not correct and prevents node
updates from working when the first argument is an ID.

Change-Id: I850cb6defb71c62121eafbf853473013e2780e2b

openstack/resource.py

@@ -434,7 +434,15 @@ class Resource(dict):
             synchronized=_synchronized)
         if self.commit_jsonpatch:
             # We need the original body to compare against
-            self._original_body = self._body.attributes.copy()
+            if _synchronized:
+                self._original_body = self._body.attributes.copy()
+            elif self.id:
+                # Never record ID as dirty.
+                self._original_body = {
+                    self._alternate_id() or 'id': self.id
+                }
+            else:
+                self._original_body = {}
 
         # TODO(mordred) This is terrible, but is a hack at the moment to ensure
         # json.dumps works. The json library does basically if not obj: and

openstack/tests/functional/baremetal/test_baremetal_node.py

@@ -48,7 +48,7 @@ class TestBareMetalNode(base.BaseBaremetalTest):
         instance_uuid = str(uuid.uuid4())
 
         node = self.conn.baremetal.update_node(node,
-                                               instance_uuid=instance_uuid)
+                                               instance_id=instance_uuid)
         self.assertEqual('new-name', node.name)
         self.assertEqual({'answer': 42}, node.extra)
         self.assertEqual(instance_uuid, node.instance_id)
@@ -59,12 +59,33 @@ class TestBareMetalNode(base.BaseBaremetalTest):
         self.assertEqual(instance_uuid, node.instance_id)
 
         node = self.conn.baremetal.update_node(node,
-                                               instance_uuid=None)
+                                               instance_id=None)
         self.assertIsNone(node.instance_id)
 
         node = self.conn.baremetal.get_node('new-name')
         self.assertIsNone(node.instance_id)
 
+    def test_node_update_by_name(self):
+        self.create_node(name='node-name', extra={'foo': 'bar'})
+        instance_uuid = str(uuid.uuid4())
+
+        node = self.conn.baremetal.update_node('node-name',
+                                               instance_id=instance_uuid,
+                                               extra={'answer': 42})
+        self.assertEqual({'answer': 42}, node.extra)
+        self.assertEqual(instance_uuid, node.instance_id)
+
+        node = self.conn.baremetal.get_node('node-name')
+        self.assertEqual({'answer': 42}, node.extra)
+        self.assertEqual(instance_uuid, node.instance_id)
+
+        node = self.conn.baremetal.update_node('node-name',
+                                               instance_id=None)
+        self.assertIsNone(node.instance_id)
+
+        node = self.conn.baremetal.get_node('node-name')
+        self.assertIsNone(node.instance_id)
+
     def test_node_create_in_enroll_provide(self):
         node = self.create_node(provision_state='enroll')
         self.node_id = node.id

openstack/tests/unit/image/v2/test_image.py

@@ -309,7 +309,7 @@ class TestImage(base.TestCase):
     def test_image_update(self):
         values = EXAMPLE.copy()
         del values['instance_uuid']
-        sot = image.Image(**values)
+        sot = image.Image.existing(**values)
         # Let the translate pass through, that portion is tested elsewhere
         sot._translate_response = mock.Mock()
 

openstack/tests/unit/test_resource.py

@@ -872,7 +872,7 @@ class TestResource(base.TestCase):
             y = resource.Body("y")
 
         the_id = "id"
-        sot = Test(id=the_id, x=1, y=2)
+        sot = Test.existing(id=the_id, x=1, y=2)
         sot.x = 3
 
         result = sot._prepare_request(requires_id=True, patch=True)
@@ -881,6 +881,22 @@ class TestResource(base.TestCase):
         self.assertEqual([{'op': 'replace', 'path': '/x', 'value': 3}],
                          result.body)
 
+    def test__prepare_request_with_patch_not_synchronized(self):
+        class Test(resource.Resource):
+            commit_jsonpatch = True
+            base_path = "/something"
+            x = resource.Body("x")
+            y = resource.Body("y")
+
+        the_id = "id"
+        sot = Test.new(id=the_id, x=1)
+
+        result = sot._prepare_request(requires_id=True, patch=True)
+
+        self.assertEqual("something/id", result.url)
+        self.assertEqual([{'op': 'add', 'path': '/x', 'value': 1}],
+                         result.body)
+
     def test__translate_response_no_body(self):
         class Test(resource.Resource):
             attr = resource.Header("attr")