Tags refactoring (#248)

Author: zeako

.gitignore

@@ -19,7 +19,7 @@ develop-eggs
 lib
 lib64
 __pycache__
-venv/
+venv*/
 
 # Installer logs
 pip-log.txt

opencensus/tags/__init__.py

@@ -11,3 +11,10 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+
+from opencensus.tags.tag import Tag
+from opencensus.tags.tag_key import TagKey
+from opencensus.tags.tag_value import TagValue
+from opencensus.tags.tag_map import TagMap
+
+__all__ = ['Tag', 'TagKey', 'TagValue', 'TagMap']

opencensus/tags/propagation/binary_serializer.py

@@ -47,12 +47,12 @@ def to_byte_array(self, tag_context):
         encoded_bytes = b''
         encoded_bytes += _VarintBytes(VERSION_ID)
         total_chars = 0
-        for tag in tag_context.tags:
-            for tag_key, tag_value in tag.items():
-                total_chars += len(tag_key)
-                total_chars += len(tag_value)
-                encoded_bytes = self._encode_tag(
-                    tag_key, tag_value, encoded_bytes)
+        for tag in tag_context:
+            tag_key, tag_value = tag
+            total_chars += len(tag_key)
+            total_chars += len(tag_value)
+            encoded_bytes = self._encode_tag(
+                tag_key, tag_value, encoded_bytes)
         if total_chars <= TAG_MAP_SERIALIZED_SIZE_LIMIT:
             return encoded_bytes
         else:  # pragma: NO COVER

opencensus/tags/tag.py

@@ -12,30 +12,19 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from opencensus.tags import tag_key
-from opencensus.tags import tag_value
+from collections import namedtuple
 
+Tag_ = namedtuple('Tag', ['key', 'value'])
 
-class Tag(object):
-    """ A tag, in the format [KEY]:[VALUE].
 
-    :type key: :class: '~opencensus.tags.tag_key.TagKey'
+class Tag(Tag_):
+    """A tag, in the format [KEY]:[VALUE].
+
+    :type key: '~opencensus.tags.tag_key.TagKey'
     :param key: Key in the tag
 
-    :type value: :class: 'opencensus.tags.tag_value.TagValue'
+    :type value: '~opencensus.tags.tag_key.TagValue'
     :param value: Value of the key in the tag.
 
     """
-    def __init__(self, key, value):
-        self._key = tag_key.TagKey(key)
-        self._value = tag_value.TagValue(value)
-
-    @property
-    def key(self):
-        """The key of the current tag"""
-        return self._key
-
-    @property
-    def value(self):
-        """The value of the key in the current tag"""
-        return self._value
+    pass

opencensus/tags/tag_key.py

@@ -12,36 +12,23 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from opencensus.tags.validation import is_valid_tag_name
 
-class TagKey(object):
-    """ A tag key with a property name
+_TAG_NAME_ERROR = \
+    'tag name must not be empty,' \
+    'no longer than 255 characters and of ascii values between 32 - 126'
 
-    :type name: str
-    :param name: The name of the key
 
-    """
-    def __init__(self, name):
-        self._name = name
+class TagKey(str):
+    """A tag key with a property name"""
 
-    @property
-    def name(self):
-        """The name of the current key"""
-        return self._name
-
-    def is_valid_name(self, name):
-        """Checks if the name of the key is valid
+    def __new__(cls, name):
+        """Create and return a new tag key
 
         :type name: str
-        :param name: name to check
-
-        :rtype: bool
-        :returns: True if it valid, else returns False
+        :param name: The name of the key
+        :return: TagKey
         """
-        if (len(name) > 0) and (len(name) <= 255):
-            if (all(ord(char) < 126 for char in name) and
-                    all(ord(char) > 32 for char in name)):
-                return True
-            else:
-                return False
-        else:
-            return False
+        if not is_valid_tag_name(name):
+            raise ValueError(_TAG_NAME_ERROR)
+        return super(TagKey, cls).__new__(cls, name)

opencensus/tags/tag_map.py

@@ -12,6 +12,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from collections import OrderedDict
+
+from opencensus.tags.tag_key import TagKey
+from opencensus.tags.tag_value import TagValue
+
 
 class TagMap(object):
     """ A tag map is a map of tags from key to value
@@ -20,21 +25,12 @@ class TagMap(object):
     :param tags: a list of tags
 
     """
-    def __init__(self, tags=None):
-        self._map = {}
-        if tags is not None:
-            self.tags = tags
-            for tag in self.tags:
-                for tag_key, tag_value in tag.items():
-                    self._map[tag_key] = tag_value
 
-        else:
-            self._map = {}
+    def __init__(self, tags=None):
+        self.map = OrderedDict(tags if tags else [])
 
-    @property
-    def map(self):
-        """The current map of tags"""
-        return self._map
+    def __iter__(self):
+        return self.map.items().__iter__()
 
     def insert(self, key, value):
         """Inserts a key and value in the map if the map does not already
@@ -48,43 +44,50 @@ def insert(self, key, value):
         the value to insert into the tag map
 
         """
-        if key not in self._map:
-            self._map[key] = value
+        if key in self.map:
+            return
+
+        try:
+            tag_key = TagKey(key)
+            tag_val = TagValue(value)
+            self.map[tag_key] = tag_val
+        except ValueError:
+            raise
 
     def delete(self, key):
-        """ Deletes a tag from the map if the key is in the map
+        """Deletes a tag from the map if the key is in the map
 
-        :type key: str
+        :type key: :class: '~opencensus.tags.tag_key.TagKey'
         :param key: A string representing a possible tag key
 
         :returns: the value of the key in the dictionary if it is in there,
                   or None if it is not.
         """
-        self._map.pop(key, None)
+        self.map.pop(key, None)
 
     def update(self, key, value):
-        """ Updates the map by updating the value of a key
+        """Updates the map by updating the value of a key
 
         :type key: :class: '~opencensus.tags.tag_key.TagKey'
         :param key: A tag key to be updated
 
-        :type value: str
+        :type value: :class: '~opencensus.tags.tag_value.TagValue'
         :param value: The value to update the key to in the map
 
         """
-        if key in self._map:
-            self._map[key] = value
+        if key in self.map:
+            self.map[key] = value
 
     def tag_key_exists(self, key):
-        """ Checking if the tag key exists in the map
+        """Checking if the tag key exists in the map
 
-        :type key: str
+        :type key: '~opencensus.tags.tag_key.TagKey'
         :param key: A string to check to see if that is a key in the map
 
         :returns: True if the key is in map, False is it is not
 
         """
-        return key in self._map
+        return key in self.map
 
     def get_value(self, key):
         """ Gets the value of the key passed in if the key exists in the map
@@ -95,8 +98,7 @@ def get_value(self, key):
         :returns: A KeyError if the value is None, else returns the value
 
         """
-        value = self._map.get(key, None)
-        if value is None:
-            raise KeyError('Key is not in map.')
-
-        return value
+        try:
+            return self.map[key]
+        except KeyError:
+            raise KeyError('key is not in map')

opencensus/tags/tag_value.py

@@ -12,37 +12,23 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from opencensus.tags.validation import is_valid_tag_value
 
-class TagValue(object):
-    """ The value of a tag
+_TAG_VALUE_ERROR = \
+    'tag value must not be longer than 255 characters ' \
+    'and of ascii values between 32 - 126'
 
-    :type value: str
-    :param value: A string representing the value of a key in a tag
 
-    """
-    def __init__(self, value):
-        self._value = value
+class TagValue(str):
+    """The value of a tag"""
 
-    @property
-    def value(self):
-        """The current value"""
-        return self._value
-
-    def is_valid_value(self, value):
-        """ Checks if the value if valid
+    def __new__(cls, value):
+        """Create and return a new tag value
 
         :type value: str
-        :param value: the value to be checked
-
-        :rtype: bool
-        :returns: True if valid, if not, False.
-
+        :param value: A string representing the value of a key in a tag
+        :return: TagValue
         """
-        if len(value) <= 255:
-            if (all(ord(char) < 126 for char in value) and
-                    all(ord(char) > 32 for char in value)):
-                return True
-            else:
-                return False
-        else:
-            return False
+        if not is_valid_tag_value(value):
+            raise ValueError(_TAG_VALUE_ERROR)
+        return super(TagValue, cls).__new__(cls, value)

opencensus/tags/validation.py

@@ -0,0 +1,42 @@
+# Copyright 2018, OpenCensus Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+def is_legal_chars(value):
+    return all(32 <= ord(char) <= 126 for char in value)
+
+
+def is_valid_tag_name(name):
+    """Checks if the name of a tag key is valid
+
+    :type name: str
+    :param name: name to check
+
+    :rtype: bool
+    :returns: True if it valid, else returns False
+    """
+    return is_legal_chars(name) if 0 < len(name) <= 255 else False
+
+
+def is_valid_tag_value(value):
+    """Checks if the value is valid
+
+    :type value: str
+    :param value: the value to be checked
+
+    :rtype: bool
+    :returns: True if valid, if not, False.
+
+    """
+    return is_legal_chars(value) if len(value) <= 255 else False

tests/unit/tags/propagation/test_binary_serializer.py

@@ -14,6 +14,7 @@
 
 import unittest
 
+from opencensus.tags import Tag, TagKey, TagValue
 from opencensus.tags.propagation import binary_serializer
 
 
@@ -43,12 +44,11 @@ def test_from_byte_array(self):
 
     def test_to_byte_array(self):
         from opencensus.tags.tag_map import TagMap
-        from collections import OrderedDict
 
-        tag_map = OrderedDict(
-            [('key1', 'val1'), ('key2', 'val2'),
-             ('key3', 'val3'), ('key4', 'val4')])
-        tags = [tag_map]
+        tags = [Tag(TagKey('key1'), TagValue('val1')),
+                Tag(TagKey('key2'), TagValue('val2')),
+                Tag(TagKey('key3'), TagValue('val3')),
+                Tag(TagKey('key4'), TagValue('val4'))]
         tag_context = TagMap(tags=tags)
         propagator = binary_serializer.BinarySerializer()
         binary = propagator.to_byte_array(tag_context)

tests/unit/tags/test_tag.py

@@ -13,15 +13,15 @@
 # limitations under the License.
 
 import unittest
-from opencensus.tags import tag as tag_module
+from opencensus.tags import Tag, TagKey, TagValue
 
 
 class TestTag(unittest.TestCase):
 
     def test_constructor(self):
         key = 'key1'
         value = 'value1'
-        tag = tag_module.Tag(key=key, value=value)
+        tag = Tag(key=TagKey(key), value=TagValue(value))
 
-        self.assertEqual(tag.key.name, key)
-        self.assertEqual(tag.value.value, value)
+        self.assertEqual(tag.key, key)
+        self.assertEqual(tag.value, value)