From e3a3adaae26d9be7d8648f9400cfa8cd8d601ac6 Mon Sep 17 00:00:00 2001
From: Mike Fedosin <mikhail.fedosin.ext@nokia.com>
Date: Tue, 13 Jun 2017 17:47:09 +0300
Subject: [PATCH] Fix creation of artifact with ':' in names.

Currently to prevent name/version colisions we look
for artifact with the same name/version before creation.
To do this we use simple 'list' operation. Unfortunately,
if name or version contains ':', it is considered as filter_op
delimeter and Glare returns 400 for these cases.

This code fixes this and adds explicit operator 'eq:' for each
value.

Change-Id: I7760019cdbe1f6e8c5bf1c3b22a3cf07392aff19
Closes-bug: #1697698
---
 glare/objects/base.py               |  4 +--
 glare/tests/unit/api/test_create.py | 42 ++++++++++++++++++++++-------
 glare/tests/unit/api/test_update.py |  8 ++++++
 3 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/glare/objects/base.py b/glare/objects/base.py
index 0f6e376..828a1bc 100644
--- a/glare/objects/base.py
+++ b/glare/objects/base.py
@@ -280,9 +280,9 @@ class BaseArtifact(base.VersionedObject):
         :param is_public: flag that indicates to search artifact globally
         """
         if version is not None and name not in (None, ""):
-            filters = [('name', name), ('version', version)]
+            filters = [('name', 'eq:' + name), ('version', 'eq:' + version)]
             if is_public is False:
-                filters.extend([('owner', context.tenant),
+                filters.extend([('owner', 'eq:' + context.tenant),
                                 ('visibility', 'private')])
             else:
                 filters.extend([('visibility', 'public')])
diff --git a/glare/tests/unit/api/test_create.py b/glare/tests/unit/api/test_create.py
index 8393b0d..36b230c 100644
--- a/glare/tests/unit/api/test_create.py
+++ b/glare/tests/unit/api/test_create.py
@@ -21,17 +21,37 @@ class TestArtifactCreate(base.BaseTestArtifactAPI):
     """Test Glare artifact creation."""
 
     def test_create_artifact_minimal(self):
-        values = {'name': 'ttt'}
 
+        for name in ['ttt', 'tt:t', 'tt t', 'tt: t', 'tt,t']:
+            values = {'name': name}
+
+            res = self.controller.create(self.req, 'sample_artifact', values)
+            self.assertEqual(name, res['name'])
+            self.assertEqual('0.0.0', res['version'])
+            self.assertEqual(self.users['user1']['tenant_id'], res['owner'])
+            self.assertEqual('drafted', res['status'])
+            self.assertEqual('private', res['visibility'])
+            self.assertEqual('', res['description'])
+            self.assertEqual({}, res['metadata'])
+            self.assertEqual([], res['tags'])
+
+    def test_create_artifact_with_version(self):
+        values = {'name': 'name', 'version': '1.0'}
         res = self.controller.create(self.req, 'sample_artifact', values)
-        self.assertEqual('ttt', res['name'])
-        self.assertEqual('0.0.0', res['version'])
-        self.assertEqual(self.users['user1']['tenant_id'], res['owner'])
-        self.assertEqual('drafted', res['status'])
-        self.assertEqual('private', res['visibility'])
-        self.assertEqual('', res['description'])
-        self.assertEqual({}, res['metadata'])
-        self.assertEqual([], res['tags'])
+        self.assertEqual('name', res['name'])
+        self.assertEqual('1.0.0', res['version'])
+
+        values = {'name': 'name', 'version': '1:0'}
+        res = self.controller.create(self.req, 'sample_artifact', values)
+        self.assertEqual('1.0.0-0', res['version'])
+
+        values = {'name': 'name', 'version': '1:0:0'}
+        res = self.controller.create(self.req, 'sample_artifact', values)
+        self.assertEqual('1.0.0-0-0', res['version'])
+
+        values = {'name': 'name', 'version': '2:0-0'}
+        res = self.controller.create(self.req, 'sample_artifact', values)
+        self.assertEqual('2.0.0-0-0', res['version'])
 
     def test_create_artifact_with_fields(self):
         values = {'name': 'ttt', 'version': '1.0',
@@ -68,6 +88,10 @@ class TestArtifactCreate(base.BaseTestArtifactAPI):
         self.assertRaises(exc.BadRequest, self.controller.create,
                           self.req, 'sample_artifact', values)
 
+        values = {'name': 'test', 'version': ':'}
+        self.assertRaises(exc.BadRequest, self.controller.create,
+                          self.req, 'sample_artifact', values)
+
         values = {'name': '', 'version': '1.0'}
         self.assertRaises(exc.BadRequest, self.controller.create,
                           self.req, 'sample_artifact', values)
diff --git a/glare/tests/unit/api/test_update.py b/glare/tests/unit/api/test_update.py
index e801a62..a45119b 100644
--- a/glare/tests/unit/api/test_update.py
+++ b/glare/tests/unit/api/test_update.py
@@ -104,6 +104,10 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI):
         res = self.update_with_values(changes)
         self.assertEqual('1.0', res['name'])
 
+        changes = [{'op': 'replace', 'path': '/name', 'value': "tt:t"}]
+        res = self.update_with_values(changes)
+        self.assertEqual('tt:t', res['name'])
+
         # version
         changes = [{'op': 'replace', 'path': '/version', 'value': 2.0}]
         res = self.update_with_values(changes)
@@ -113,6 +117,10 @@ class TestArtifactUpdate(base.BaseTestArtifactAPI):
         res = self.update_with_values(changes)
         self.assertEqual('1.0.0-alpha', res['version'])
 
+        changes = [{'op': 'replace', 'path': '/version', 'value': '1:0'}]
+        res = self.update_with_values(changes)
+        self.assertEqual('1.0.0-0', res['version'])
+
     def test_update_deleted_artifact(self):
         # Enable delayed delete
         self.config(delayed_delete=True)