Repository: jbradberry/universe

Issue:
Create a new reference field type
Basically the equivalent of Django's `ForeignKey`.  Currently we are just using an `IntField` to hold the data.  This should probably also do something to make sure that the object on the other end doesn't get removed unexpectedly, or morph into an object with a different id.

- [x] give a reference to the `Manager` to `Entity`
- [x] add a new `Reference` field type
- [x] validate `Reference` values against the pool of entities
- [x] update the `OwnershipComponent` to use a `Reference` for the owner
- [x] fields should provide a `serialize()` method
- [x] `Reference` should serialize as `<name>_id`
- [x] `getattr()` of `<name>` of a `Reference` value on the entity should return an `Entity` object
- [x] `Entity` objects need to have the pk as an attribute
- [x] `setattr()` of a `Reference` should accept an `int` or an `Entity`, and should store the pk under `<name>_id`
- [x] `Entity.__init__()` should not do the validation step
- [x] `Entity` should provide a `validate()` method
- [x] eliminate `Entity._data`
- [x] `Entity.__init__()` should take kwargs, not a data dict
- [x] ~`Reference` should provide a way to get the collection of items that point to a given `Entity` object~ **edit:** going to pass on this one, it's a bit tricky
- [x] validate the entity pool after loading all of them
- [x] `Reference` should optionally take a list of entity types that it can accept

Discussion:

Patch:
diff --git a/universe/components.py b/universe/components.py
index dbcba73..34cf944 100644
--- a/universe/components.py
+++ b/universe/components.py
@@ -30,9 +30,8 @@ class Component(metaclass=MetaComponent):
     def serialize(self, data):
         output = {}
         self.validate(data)
-        for name, field in self._fields.items():
-            if name in data:
-                output[name] = data[name]
+        for field in self._fields.values():
+            output.update(field.serialize(data))
         return output
 
     def display(self, data):
@@ -127,7 +126,7 @@ class SpeciesComponent(Component):
 class OwnershipComponent(Component):
     _name = 'ownership'
 
-    owner_id = fields.IntField(required=False)
+    owner = fields.Reference(types=['species'], required=False)
 
 
 class PopulationComponent(Component):
diff --git a/universe/engine.py b/universe/engine.py
index 73fc86d..1325317 100644
--- a/universe/engine.py
+++ b/universe/engine.py
@@ -1,20 +1,22 @@
-from . import components, systems
+import weakref
+
+from . import components, systems, exceptions
 from .orders import (Move, CargoTransfer, Scrap, BuildInstallation, Terraform,
                      BuildStation, BuildShip, LaunchMassPacket)
 
 
 class Entity:
-    def __init__(self, registry, data):
-        self._data = data
-        self._components = registry[data['type']]
-
-        for _type, component in self._components.items():
-            component.validate(data)
+    def __init__(self, **kwargs):
+        self.__dict__.update(kwargs)
+        self._components = Entity.manager._entity_registry[kwargs['type']]
 
     def __getattr__(self, name):
         for component in self.__dict__.get('_components', {}).values():
-            if name in component._fields:
-                return self.__dict__['_data'].get(name)
+            for field in component._fields.values():
+                if name == field.data_name:
+                    return self.__dict__.get(name)
+                if name == field.name:
+                    return field.from_data(self.__dict__)
         try:
             return self.__dict__[name]
         except KeyError:
@@ -22,27 +24,40 @@ class Entity:
 
     def __setattr__(self, name, value):
         for component in self.__dict__.get('_components', {}).values():
-            if name in component._fields:
-                self.__dict__['_data'][name] = value
-                return
+            for field in component._fields.values():
+                if name in (field.data_name, field.name):
+                    try:
+                        self.__dict__[field.data_name] = field.to_data(value)
+                    except exceptions.empty:
+                        self.__dict__.pop(field.data_name, None)
+                    return
         self.__dict__[name] = value
 
     def __delattr__(self, name):
         for component in self.__dict__.get('_components', {}).values():
-            if name in component._fields:
-                del self.__dict__['_data'][name]
-                return
+            for field in component._fields.values():
+                if name in (field.data_name, field.name):
+                    self.__dict__.pop(field.data_name)
+                    return
         del self.__dict__[name]
 
     def __contains__(self, key):
         return key in self._components
 
+    def validate(self):
+        for component in self._components.values():
+            component.validate(self.__dict__)
+
     def serialize(self):
-        data = {'type': self._data['type']}
+        data = {}
         for _type, component in self._components.items():
-            data.update(component.serialize(self._data))
+            data.update(component.serialize(self.__dict__))
         return data
 
+    @classmethod
+    def register_manager(cls, manager):
+        cls.manager = weakref.proxy(manager)
+
 
 class Manager:
     def __init__(self):
@@ -74,9 +89,10 @@ class Manager:
         self._components.setdefault(_type, {})[_id] = entity
 
     def register_entity(self, _id, entity):
-        entity_obj = Entity(self._entity_registry, entity)
+        entity_obj = Entity(**entity)
+        entity_obj.pk = _id
         for component in entity_obj._components:
-            self._components.setdefault(component, {})[_id] = entity_obj
+            self.set_entity(component, _id, entity_obj)
 
     def process(self):
         for system_cls in self._systems:
@@ -86,10 +102,12 @@ class Manager:
     def import_data(self, data, updates):
         for _id, entity in (data.get('entities') or {}).items():
             self.register_entity(_id, entity)
+        for entity in self.get_entities('metadata').values():
+            entity.validate()
         self._updates = updates
 
     def export_data(self):
-        data = {_id: entity for _id, entity in self._components.get('metadata', {}).items()}
+        data = {_id: entity for _id, entity in self.get_entities('metadata').items()}
 
         return {'entities': {_id: entity.serialize() for _id, entity in data.items()}}
 
@@ -125,6 +143,7 @@ class GameState:
         self.manager.register_entity_type('species', [
             components.SpeciesComponent(),
         ])
+        Entity.register_manager(self.manager)
 
         self.new = {}
 
diff --git a/universe/exceptions.py b/universe/exceptions.py
index ead42d5..35af43d 100644
--- a/universe/exceptions.py
+++ b/universe/exceptions.py
@@ -2,3 +2,7 @@
 
 class ValidationError(Exception):
     pass
+
+
+class empty(Exception):
+    pass
diff --git a/universe/fields.py b/universe/fields.py
index d0db79d..176aefd 100644
--- a/universe/fields.py
+++ b/universe/fields.py
@@ -5,9 +5,24 @@ class Field:
     def __init__(self, required=True):
         self.required = required
 
+    @property
+    def data_name(self):
+        return self.name
+
+    def from_data(self, data):
+        return data.get(self.data_name)
+
+    def to_data(self, value):
+        return value
+
+    def serialize(self, data):
+        if self.data_name in data:
+            return {self.data_name: data[self.data_name]}
+        return {}
+
     def validate(self, data):
-        if self.required and self.name not in data:
-            raise exceptions.ValidationError(f"{self.name!r} is required.")
+        if self.required and self.data_name not in data:
+            raise exceptions.ValidationError(f"{self.data_name!r} is required.")
 
 
 class BooleanField(Field):
@@ -16,9 +31,9 @@ class BooleanField(Field):
 
     def validate(self, data):
         super().validate(data)
-        value = data[self.name]
+        value = data[self.data_name]
         if not isinstance(value, bool):
-            raise exceptions.ValidationError(f"{self.name!r} must be a boolean.")
+            raise exceptions.ValidationError(f"{self.data_name!r} must be a boolean.")
 
 
 class IntField(Field):
@@ -29,32 +44,73 @@ class IntField(Field):
 
     def validate(self, data):
         super().validate(data)
-        if self.name not in data:
+        if self.data_name not in data:
             return
-        value = data[self.name]
+        value = data[self.data_name]
         if not isinstance(value, int):
-            raise exceptions.ValidationError(f"{self.name!r} must be an integer.")
+            raise exceptions.ValidationError(f"{self.data_name!r} must be an integer.")
         if self.min is not None and value < self.min:
-            raise exceptions.ValidationError(f"{self.name!r} must be greater than or equal to {self.min}.")
+            raise exceptions.ValidationError(f"{self.data_name!r} must be greater than or equal to {self.min}.")
         if self.max is not None and value > self.max:
-            raise exceptions.ValidationError(f"{self.name!r} must be less than or equal to {self.max}.")
+            raise exceptions.ValidationError(f"{self.data_name!r} must be less than or equal to {self.max}.")
 
 
 class ListField(Field):
     def validate(self, data):
         super().validate(data)
-        if self.name not in data:
+        if self.data_name not in data:
             return
-        value = data[self.name]
+        value = data[self.data_name]
         if not isinstance(value, list):
-            raise exceptions.ValidationError(f"{self.name!r} must be a list.")
+            raise exceptions.ValidationError(f"{self.data_name!r} must be a list.")
 
 
 class CharField(Field):
     def validate(self, data):
         super().validate(data)
-        if self.name not in data:
+        if self.data_name not in data:
             return
-        value = data[self.name]
+        value = data[self.data_name]
         if not isinstance(value, str):
-            raise exceptions.ValidationError(f"{self.name!r} must be a string.")
+            raise exceptions.ValidationError(f"{self.data_name!r} must be a string.")
+
+
+class Reference(Field):
+    def __init__(self, types=None, **kwargs):
+        super().__init__(**kwargs)
+        self.types = types
+
+    @property
+    def data_name(self):
+        return f'{self.name}_id'
+
+    def from_data(self, data):
+        value = data.get(self.data_name)
+        if value is None:
+            return None
+
+        from .engine import Entity
+        return Entity.manager.get_entity('metadata', value)
+
+    def to_data(self, value):
+        from .engine import Entity
+        if isinstance(value, Entity):
+            return value.pk
+        if value is None:
+            raise exceptions.empty
+        return value
+
+    def validate(self, data):
+        super().validate(data)
+        if self.data_name not in data:
+            return
+        value = data[self.data_name]
+        if not isinstance(value, int):
+            raise exceptions.ValidationError(f"{self.data_name!r} must be an integer.")
+
+        from .engine import Entity
+        entity = Entity.manager.get_entity('metadata', value)
+        if entity is None:
+            raise exceptions.ValidationError(f"{self.data_name!r} is not an existing entity.")
+        if self.types is not None and entity.type not in self.types:
+            raise exceptions.ValidationError(f"{self.data_name!r} cannot point to an entity of this type.")


Test patch:
diff --git a/tests/test_components.py b/tests/test_components.py
index e38906a..c845fe0 100644
--- a/tests/test_components.py
+++ b/tests/test_components.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 import unittest
 
-from universe import components, fields, exceptions
+from universe import components, fields, engine, exceptions
 
 
 class ComponentTestCase(unittest.TestCase):
@@ -196,3 +196,74 @@ class SpeciesComponentTestCase(unittest.TestCase):
                 str(e.exception),
                 f"'{fname}_min' and '{fname}_max' must be set if '{fname}_immune' is false."
             )
+
+
+class OwnershipComponentTestCase(unittest.TestCase):
+    def setUp(self):
+        self.manager = engine.Manager()
+        self.manager._entity_registry = {
+            'species': {
+                'metadata': components.MetadataComponent(),
+            },
+            'planet': {
+                'metadata': components.MetadataComponent(),
+                'ownership': components.OwnershipComponent(),
+            },
+        }
+        engine.Entity.register_manager(self.manager)
+
+    def test_null_pointer(self):
+        self.manager.register_entity(0, {'type': 'planet'})
+
+        planet = self.manager.get_entity('metadata', 0)
+        self.assertIsNone(planet.owner)
+
+    def test_entity_reference(self):
+        self.manager.register_entity(0, {'type': 'species'})
+        self.manager.register_entity(1, {'type': 'planet', 'owner_id': 0})
+
+        planet = self.manager.get_entity('metadata', 1)
+
+        self.assertIsInstance(planet.owner, engine.Entity)
+        self.assertEqual(planet.owner.serialize(), {'type': 'species'})
+
+    def test_set_reference(self):
+        self.manager.register_entity(0, {'type': 'species'})
+        self.manager.register_entity(1, {'type': 'species'})
+        self.manager.register_entity(2, {'type': 'planet'})
+
+        planet = self.manager.get_entity('metadata', 2)
+        self.assertIsNone(planet.owner)
+        self.assertIsNone(planet.owner_id)
+
+        planet.owner = 0
+        self.assertIsInstance(planet.owner, engine.Entity)
+        self.assertIsInstance(planet.owner_id, int)
+        self.assertEqual(planet.owner.pk, 0)
+        self.assertEqual(planet.owner_id, 0)
+
+        planet.owner = self.manager.get_entity('metadata', 1)
+        self.assertIsInstance(planet.owner, engine.Entity)
+        self.assertIsInstance(planet.owner_id, int)
+        self.assertEqual(planet.owner.pk, 1)
+        self.assertEqual(planet.owner_id, 1)
+
+        planet.owner = None
+        self.assertIsNone(planet.owner)
+        self.assertIsNone(planet.owner_id)
+
+    def test_wrong_owner_type(self):
+        self.manager.register_entity(0, {'type': 'planet'})
+        self.manager.register_entity(1, {'type': 'planet'})
+
+        planet0 = self.manager.get_entity('metadata', 0)
+        planet1 = self.manager.get_entity('metadata', 1)
+
+        self.assertIsNone(planet0.validate())
+        self.assertIsNone(planet1.validate())
+
+        planet1.owner = planet0
+        with self.assertRaises(exceptions.ValidationError) as e:
+            planet1.validate()
+
+        self.assertEqual(str(e.exception), "'owner_id' cannot point to an entity of this type.")
diff --git a/tests/test_engine.py b/tests/test_engine.py
index 480a4ac..c935bea 100644
--- a/tests/test_engine.py
+++ b/tests/test_engine.py
@@ -1,6 +1,39 @@
 import unittest
 
-from universe import engine
+from universe import engine, exceptions
+
+
+class EntityTestCase(unittest.TestCase):
+    def test_invalid_ownership(self):
+        state = {
+            'turn': 2500,
+            'width': 1000,
+            'entities': {
+                1: {
+                    'type': 'planet',
+                    'x': 300,
+                    'y': 600,
+                    'gravity': 27,
+                    'temperature': 36,
+                    'radiation': 45,
+                    'ironium_conc': 67,
+                    'boranium_conc': 78,
+                    'germanium_conc': 82,
+                    'ironium': 20,
+                    'boranium': 30,
+                    'germanium': 40,
+                    'queue': [],
+                    'owner_id': 0,  # Not an entity in the pool.
+                    'population': 1000,
+                }
+            }
+        }
+
+        S = engine.GameState(state, {})
+        with self.assertRaises(exceptions.ValidationError) as e:
+            results = S.generate()
+
+        self.assertEqual(str(e.exception), "'owner_id' is not an existing entity.")
 
 
 class PersistenceTestCase(unittest.TestCase):
diff --git a/tests/test_fields.py b/tests/test_fields.py
index d8b67ef..058dc95 100644
--- a/tests/test_fields.py
+++ b/tests/test_fields.py
@@ -163,3 +163,27 @@ class CharFieldTestCase(unittest.TestCase):
         field.name = 'name'
 
         self.assertIsNone(field.validate({'name': "Human"}))
+
+
+class ReferenceTestCase(unittest.TestCase):
+    def test_not_required(self):
+        field = fields.Reference(required=False)
+        field.name = 'owner'
+
+        self.assertIsNone(field.validate({}))
+
+    def test_explicit_required(self):
+        field = fields.Reference(required=True)
+        field.name = 'owner'
+
+        with self.assertRaises(exceptions.ValidationError) as e:
+            field.validate({})
+        self.assertEqual(str(e.exception), "'owner_id' is required.")
+
+    def test_bad_type(self):
+        field = fields.Reference(required=True)
+        field.name = 'owner'
+
+        with self.assertRaises(exceptions.ValidationError) as e:
+            field.validate({'owner_id': 'a'})
+        self.assertEqual(str(e.exception), "'owner_id' must be an integer.")
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 93b9590..df720b8 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -5,7 +5,8 @@ from universe import components, engine, utils
 
 class PlanetValueTestCase(unittest.TestCase):
     def test_values(self):
-        registry = {
+        manager = engine.Manager()
+        manager._entity_registry = {
             'species': {
                 'metadata': components.MetadataComponent(),
                 'species': components.SpeciesComponent(),
@@ -15,8 +16,9 @@ class PlanetValueTestCase(unittest.TestCase):
                 'environment': components.EnvironmentComponent(),
             },
         }
-        species = engine.Entity(
-            registry,
+        engine.Entity.register_manager(manager)
+
+        species = engine.Entity(**
             {'type': 'species',
              'name': 'Human',
              'plural_name': 'Humans',
@@ -955,8 +957,7 @@ class PlanetValueTestCase(unittest.TestCase):
         ]
 
         for g, t, r, value in data:
-            planet = engine.Entity(
-                registry,
+            planet = engine.Entity(**
                 {'type': 'planet',
                  'gravity': g,
                  'temperature': t,