From e0efab0b28cb039c14b44bbb3f04e6079c1572bf Mon Sep 17 00:00:00 2001
From: Lucas Alvares Gomes <lucasagomes@gmail.com>
Date: Tue, 5 Aug 2014 16:40:13 +0100
Subject: [PATCH] Fix self.fields on API Port object

All fields from objects.Port were being added to self.fields in the
API Port object. Because of this, when someone would POST, there'd be an
entry for {'id': None} in the dictionary passed to dbapi.create_port(). We
should only set fields we're exposing.

This also required fixing PATCH to not try to look at fields not set on
the API Port object when mapping to objects.Port.

Change-Id: I7f163d42d1298ce4ba62b1b7d637fb0a4e3409ce
---
 ironic/api/controllers/v1/port.py | 28 ++++++++++++++++++++++------
 ironic/tests/api/v1/test_ports.py | 11 +++++++++++
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py
index ef04da0736..17b7498f70 100644
--- a/ironic/api/controllers/v1/port.py
+++ b/ironic/api/controllers/v1/port.py
@@ -87,12 +87,23 @@ class Port(base.APIBase):
     "A list containing a self link and associated port links"
 
     def __init__(self, **kwargs):
-        self.fields = objects.Port.fields.keys()
+        self.fields = []
+        fields = list(objects.Port.fields.keys())
         # NOTE(lucasagomes): node_uuid is not part of objects.Port.fields
         #                    because it's an API-only attribute
-        self.fields.append('node_uuid')
-        for k in self.fields:
-            setattr(self, k, kwargs.get(k))
+        fields.append('node_uuid')
+        for field in fields:
+            # Skip fields we do not expose.
+            if not hasattr(self, field):
+                continue
+            self.fields.append(field)
+            setattr(self, field, kwargs.get(field))
+
+        # NOTE(lucasagomes): node_id is an attribute created on-the-fly
+        # by _set_node_uuid(), it needs to be present in the fields so
+        # that as_dict() will contain node_id field when converting it
+        # before saving it in the database.
+        self.fields.append('node_id')
         setattr(self, 'node_uuid', kwargs.get('node_id'))
 
     @classmethod
@@ -307,8 +318,13 @@ class PortsController(rest.RestController):
 
         # Update only the fields that have changed
         for field in objects.Port.fields:
-            if rpc_port[field] != getattr(port, field):
-                rpc_port[field] = getattr(port, field)
+            try:
+                patch_val = getattr(port, field)
+            except AttributeError:
+                # Ignore fields that aren't exposed in the API
+                continue
+            if rpc_port[field] != patch_val:
+                rpc_port[field] = patch_val
 
         rpc_node = objects.Node.get_by_id(pecan.request.context,
                                           rpc_port.node_id)
diff --git a/ironic/tests/api/v1/test_ports.py b/ironic/tests/api/v1/test_ports.py
index 3b345f9442..88e2b96e2d 100644
--- a/ironic/tests/api/v1/test_ports.py
+++ b/ironic/tests/api/v1/test_ports.py
@@ -508,6 +508,17 @@ class TestPost(base.FunctionalTest):
         self.assertEqual(urlparse.urlparse(response.location).path,
                          expected_location)
 
+    def test_create_port_doesnt_contain_id(self):
+        with mock.patch.object(self.dbapi, 'create_port',
+                               wraps=self.dbapi.create_port) as cp_mock:
+            pdict = post_get_test_port(extra={'foo': 123})
+            self.post_json('/ports', pdict)
+            result = self.get_json('/ports/%s' % pdict['uuid'])
+            self.assertEqual(pdict['extra'], result['extra'])
+            cp_mock.assert_called_once_with(mock.ANY)
+            # Check that 'id' is not in first arg of positional args
+            self.assertNotIn('id', cp_mock.call_args[0][0])
+
     def test_create_port_generate_uuid(self):
         pdict = post_get_test_port()
         del pdict['uuid']