adding 'id' to block, making 'name' optional, having 'id' play former name-uniqueness role

nio/block/base.py

@@ -8,14 +8,14 @@
     DEFAULT_TERMINAL
 from nio.command import command
 from nio.command.holder import CommandHolder
-from nio.router.base import BlockRouter
 from nio.properties import PropertyHolder, StringProperty, \
     VersionProperty, SelectProperty
+from nio.router.base import BlockRouter
+from nio.signal.base import Signal
+from nio.signal.status import BlockStatusSignal
 from nio.util.logging import get_nio_logger
 from nio.util.logging.levels import LogLevel
 from nio.util.runner import Runner
-from nio.signal.status import BlockStatusSignal
-from nio.signal.base import Signal
 
 
 @command('properties')
@@ -25,7 +25,8 @@ class Base(PropertyHolder, CommandHolder, Runner):
 
     version = VersionProperty(version='0.0.0')
     type = StringProperty(title="Type", visible=False, readonly=True)
-    name = StringProperty(title="Name", visible=False)
+    id = StringProperty(title="Id", visible=False, allow_none=False)
+    name = StringProperty(title="Name", visible=False, allow_none=True)
     log_level = SelectProperty(enum=LogLevel,
                                title="Log Level", default="NOTSET")
 
@@ -85,7 +86,7 @@ def configure(self, context):
         # verify that block properties are valid
         self.validate()
 
-        self.logger = get_nio_logger(self.name())
+        self.logger = get_nio_logger(self.label())
         self.logger.setLevel(self.log_level())
         self._service_name = context.service_name
 
@@ -151,6 +152,7 @@ def notify_management_signal(self, signal):
         if isinstance(signal, BlockStatusSignal):
             # set service block is part of
             signal.service_name = self._service_name
+            signal.block_id = self.id()
             signal.block_name = self.name()
             self.status.add(signal.status)
         self._block_router.notify_management_signal(self, signal)
@@ -221,6 +223,19 @@ def is_output_valid(self, output_id):
         """
         return output_id in [o.id for o in self.__class__.outputs()]
 
+    def label(self, include_id=False):
+        """ Provides a label to a block based on name and id properties
+
+        Args:
+            include_id: whether id is to be included in label 
+        """
+        if self.name():
+            if include_id:
+                return "{}-{}".format(self.name(), self.id())
+            else:
+                return self.name()
+        return self.id()
+
 
 @input(DEFAULT_TERMINAL, default=True, label="default")
 @output(DEFAULT_TERMINAL, default=True, label="default")

nio/block/mixins/persistence/persistence.py

@@ -1,6 +1,6 @@
-from nio.properties import TimeDeltaProperty, BoolProperty
 from nio.modules.persistence import Persistence as PersistenceModule
 from nio.modules.scheduler import Job
+from nio.properties import TimeDeltaProperty, BoolProperty
 
 
 class Persistence(object):
@@ -39,14 +39,14 @@ def persisted_values(self):
     def _load(self):
         """ Load the values from persistence
 
-        Item is loaded from persistence using block name, once item
+        Item is loaded from persistence using block id, once item
         is loaded, all persisted values are examined against loaded
         item, if it exists, such value is then set as an attribute
         in the block instance
         """
         self.logger.debug("Loading from persistence")
         # load whole item from persistence
-        item = self._persistence.load(self.name(), default={})
+        item = self._persistence.load(self.id(), default={})
         for persisted_var in self.persisted_values():
             if persisted_var in item:
                 self.logger.debug("Loaded value {} for attribute {}".format(
@@ -62,12 +62,12 @@ def _save(self):
         # to be persisted into a dictionary
         item = {persisted_var: getattr(self, persisted_var)
                 for persisted_var in self.persisted_values()}
-        # save generated dictionary under block's name
-        self._persistence.save(item, self.name())
+        # save generated dictionary under block's id
+        self._persistence.save(item, self.id())
 
     def configure(self, context):
         super().configure(context)
-        # Create a persistence object using the block's name
+        # Create a persistence object using the block's id
         self._persistence = PersistenceModule()
         if self.load_from_persistence():
             self._load()

nio/block/mixins/persistence/tests/test_persistence.py

@@ -1,8 +1,9 @@
 from datetime import timedelta
 from unittest.mock import MagicMock, patch
+
+from nio.block.base import Block
 from nio.block.mixins.persistence.persistence import Persistence
 from nio.modules.persistence import Persistence as PersistenceModule
-from nio.block.base import Block
 from nio.testing.block_test_case import NIOBlockTestCase
 
 
@@ -27,13 +28,13 @@ def get_test_modules(self):
     def test_saves_properly(self):
         """ Tests that the mixin saves the right values """
         block = PersistingBlock()
-        self.configure_block(block, {"name": "test_block"})
+        self.configure_block(block, {"id": "test_block"})
         block.start()
         # Stop the block to initiate the save using values specified
         # in block's constructor
         block.stop()
 
-        item = block._persistence.load('test_block')
+        item = block._persistence.load(block.id())
         # Make sure the right data was saved
         self.assertEqual(len(item), 2)
         self.assertEqual(item['_to_be_saved'], 'value')
@@ -44,12 +45,12 @@ def test_loads_properly(self):
         block = PersistingBlock()
         self.configure_block(block, {
             'load_from_persistence': True,
-            'name': "test_block"
+            'id': 'test_block'
         })
         block._persistence.save({
             "_to_be_saved": "saved value 1",
             "_to_be_saved_again": "saved value 2"
-        }, "test_block")
+        }, block.id())
         # Force the load now - it happened in configure too, but we hadn't
         # overwritten the values yet
         block._load()
@@ -107,59 +108,19 @@ def test_block_loading_from_persistence(self):
         can be picked up by block's persistence
         """
 
-        # save data under 'test_block' which is the name that block will have
+        block = PersistingBlock()
+        block.id = 'test_block'
+        # save data under block id
         PersistenceModule().save(
             {'_to_be_saved': 3,
              '_to_be_saved_again': 4},
-            'test_block')
+            block.id())
 
-        block = PersistingBlock()
         # make block load from persistence
         self.configure_block(block,
-                             {"name": "test_block",
+                             {'id': 'test_block',
                               'load_from_persistence': True
                               })
         # assert that data matches what was persisted outside of block
         self.assertEqual(block._to_be_saved, 3)
         self.assertEqual(block._to_be_saved_again, 4)
-
-    def test_two_blocks_persistence_sharing(self):
-        """ Tests that different block instances with same name access same values
-        """
-        block1 = PersistingBlock()
-        self.configure_block(block1, {
-            'load_from_persistence': True,
-            'name': "test_block"
-        })
-        block1._to_be_saved = '1'
-        block1._to_be_saved_again = '2'
-        block1._save()
-
-        block2 = PersistingBlock()
-        self.configure_block(block2, {
-            'load_from_persistence': True,
-            'name': "test_block"
-        })
-
-        # Make sure the new data was loaded into the right variables
-        # on the new block with same name
-        self.assertEqual(block2._to_be_saved, '1')
-        self.assertEqual(block2._to_be_saved_again, '2')
-
-        # create a block with a different name and assert that it does not
-        # pickup the new values
-        block3 = PersistingBlock()
-        self.configure_block(block3, {
-            'load_from_persistence': True,
-            'name': "test_block3"
-        })
-        # Values loaded must be default ones since a block with this
-        # name is not persisted
-        self.assertEqual(block3._to_be_saved, 'value')
-        self.assertEqual(block3._to_be_saved_again, 'another value')
-
-        # assert that it did not affect values on block1 nor block2
-        self.assertEqual(block1._to_be_saved, '1')
-        self.assertEqual(block1._to_be_saved_again, '2')
-        self.assertEqual(block2._to_be_saved, '1')
-        self.assertEqual(block2._to_be_saved_again, '2')

nio/block/tests/test_block_base.py

@@ -1,15 +1,16 @@
 from unittest.mock import patch, Mock
+
 from nio.block.base import Block
-from nio.block.terminator_block import TerminatorBlock
-from nio.block.generator_block import GeneratorBlock
 from nio.block.context import BlockContext
+from nio.block.generator_block import GeneratorBlock
 from nio.block.terminals import DEFAULT_TERMINAL
+from nio.block.terminator_block import TerminatorBlock
 from nio.properties.exceptions import AllowNoneViolation
-from nio.signal.base import Signal
 from nio.router.base import BlockRouter
 from nio.router.context import RouterContext
-from nio.testing.test_case import NIOTestCaseNoModules
+from nio.signal.base import Signal
 from nio.signal.status import BlockStatusSignal
+from nio.testing.test_case import NIOTestCaseNoModules
 from nio.util.runner import RunnerStatus
 
 
@@ -20,10 +21,9 @@ def test_configure(self):
         blk = Block()
         blk.configure(BlockContext(
             BlockRouter(),
-            {"name": "BlockName", "log_level": "WARNING"},
-            {}))
+            {"id": "BlockId", "log_level": "WARNING"}))
         # Make sure the name property got set properly
-        self.assertEqual(blk.name(), "BlockName")
+        self.assertEqual(blk.id(), "BlockId")
 
     def test_invalid_configure(self):
         """Make sure a block is configured with valid information"""
@@ -36,20 +36,19 @@ class JustAnObject(object):
             # The context's block router needs to be a BlockRouter
             Block().configure(BlockContext(JustAnObject, {}))
         with self.assertRaises(AllowNoneViolation):
-            # Block needs a name
-            Block().configure(BlockContext(BlockRouter(), {"name": None}))
+            # Block needs an id
+            Block().configure(BlockContext(BlockRouter(), {"id": None}))
         with self.assertRaises(TypeError):
-            # Wrong types (like log_level not being corrrect) raise TypeError
-            Block().configure(BlockContext(BlockRouter(), {"name": "BlockName",
+            # Wrong types (like log_level not being correct) raise TypeError
+            Block().configure(BlockContext(BlockRouter(), {"id": "BlockId",
                                                            "log_level": 42}))
 
     def test_notify_management_signal(self):
         """Test the block can notify management signals properly to router """
         blk = Block()
         blk.configure(BlockContext(
             BlockRouter(),
-            {"name": "BlockName", "log_level": "WARNING"},
-            {}))
+            {"id": "BlockId", "log_level": "WARNING"}))
         my_sig = Signal({"key": "val"})
         with patch.object(blk, '_block_router') as router_patch:
             blk.notify_management_signal(my_sig)
@@ -67,8 +66,7 @@ def test_service_notify_management_signal(self):
         block_router.configure(router_context)
         blk.configure(BlockContext(
             block_router,
-            {"name": "BlockName", "log_level": "WARNING"},
-            {}))
+            {"id": "BlockId", "log_level": "WARNING"}))
         my_sig = Signal({"key": "val"})
         blk.notify_management_signal(my_sig)
         service_mgmt_signal_handler.assert_called_once_with(my_sig)
@@ -80,7 +78,7 @@ def test_notify_signals(self):
         service_name = 'service1'
         blk.configure(BlockContext(
             BlockRouter(),
-            {"name": block_name},
+            {"id": block_name},
             service_name=service_name))
 
         my_sigs = [Signal({"key": "val"})]
@@ -171,7 +169,8 @@ def test_populate_block_status_signal(self):
         service_name = 'service1'
         blk.configure(BlockContext(
             BlockRouter(),
-            {"name": block_name},
+            {"id": "block_id",
+             "name": block_name},
             service_name=service_name))
 
         warning = BlockStatusSignal(

nio/block/tests/test_block_config.py

@@ -1,7 +1,7 @@
 from nio.block.base import Block
 from nio.block.context import BlockContext
-from nio.router.base import BlockRouter
 from nio.properties import StringProperty
+from nio.router.base import BlockRouter
 from nio.testing.test_case import NIOTestCase
 
 CONFIG_KEY = "attr_1"
@@ -11,7 +11,6 @@
 class DummyBlock(Block):
     # Create a dummy block with my configurable attribute
     attr_1 = StringProperty(title="attr_1", allow_none=True)
-    # For this test only, allow name to be None
     name = StringProperty(title="name", allow_none=True)
 
 
@@ -28,8 +27,7 @@ def tearDownModules(self):
     def setUp(self):
         # Build some sample configuration (don't actually load anything)
         super().setUp()
-        self._config = {"name": "dummy_block"}
-        self._config[CONFIG_KEY] = CONFIG_VAL
+        self._config = {"id": "dummy_block", CONFIG_KEY: CONFIG_VAL}
 
     def test_load_config(self):
         """Test that a configuration can get loaded into the block"""
@@ -43,7 +41,7 @@ def test_no_load_config(self):
         """Test that with no configuration the attribute exists, but not set"""
         block = DummyBlock()
         block.configure(BlockContext(BlockRouter(),
-                                     dict()))
+                                     {"id": "BlockId"}))
 
         self.assertTrue(hasattr(block, CONFIG_KEY))
         self.assertEqual(getattr(block, CONFIG_KEY)(), None)

nio/project/project.py

@@ -16,7 +16,7 @@ class Project(object):
     def __init__(self):
         super().__init__()
         # A project has configured blocks. It is stored as a dictionary
-        # where the key is the block name and the value is a Block instance
+        # where the key is the block id and the value is a Block instance
         self.blocks = dict()
 
         # A project has configured services. It is stored as a dictionary

nio/properties/holder.py

@@ -193,14 +193,15 @@ def get_class_properties(cls):
         return getattr(cls, class_attribute)
 
     def _process_and_log_version(self, class_properties, properties, logger):
-        name = properties.get("name", "")
+        # get either 'id' or 'name' property of this holder
+        id = properties.get("id", properties.get("name", ""))
         try:
             self._handle_versions(class_properties, properties)
         except OlderThanMinVersion as e:
             if logger:
                 logger.warning('Instance {0} version: {1} is older than'
                                ' minimum: {2}'.format
-                               (name, e.instance_version, e.min_version))
+                               (id, e.instance_version, e.min_version))
         except (NoClassVersion, NoInstanceVersion):
             # pass on classes with no version info.
             # pass on instances with no version info in their config.