Merge pull request #39 from uber/ca/add_build_strategy

Add ability to use custom builder

.gitignore

@@ -1,4 +1,5 @@
 *.py[co]
+tags
 
 # Packages
 *.egg

CHANGES.rst

@@ -1,6 +1,33 @@
 Changelog for Charlatan
 =======================
 
+0.4.0 (unreleased)
+------------------
+
+- **Breaking change**: ``get_builder`` and ``delete_builder`` arguments were
+  added to :py:class:`charlatan.FixturesManager`.
+- **Breaking change**: ``delete_instance``, ``save_instance`` methods were
+  deleted in favor of using builders (see below).
+- **Breaking change**: ``fields`` argument on
+  :py:class:`charlatan.fixture.Fixture` and fixtures collection class has
+  been renamed ``overrides`` for consistency reasons.
+- **Breaking change**: ``attrs`` argument on
+  :py:class:`charlatan.FixturesManager` been renamed ``overrides`` for
+  consistency reasons.
+- **Breaking change**: deleting fixtures will not return anything. It used to
+  return the fixture or list of fixtures that were successfully deleted. It has
+  been removed to apply the command query separation pattern. There are other
+  ways to check which fixtures are installed, and hooks or builders can be used
+  to customize deletion.
+- **Breaking change**: ``do_not_save`` and ``do_not_delete`` arguments have
+  been removed from all functions, in favor of using builders.
+- The notion of :py:class:`charlatan.builder.Builder` was added. This allows
+  customizing how fixtures are instantiated and installed. A ``builder``
+  argument has been added to most method dealing with getting, installing or
+  deleting fixtures. Sane defaults have been added in most places.
+- Improve documentation about using pytest with charlatan.
+- Fix bug preventing being able to load multiple fixtures file.
+
 0.3.12 (2015-01-14)
 -------------------
 

charlatan/builder.py

@@ -0,0 +1,93 @@
+from charlatan.utils import is_sqlalchemy_model
+
+
+class Builder(object):
+
+    def __call__(self, fixtures, klass, params, **kwargs):
+        """Build a fixture.
+
+        :param FixturesManager fixtures:
+        :param klass: the fixture's class (``model`` in the definition file)
+        :param params: the fixture's params (``fields`` in the definition
+            file)
+        :param dict kwargs:
+
+        ``kwargs`` allows passing arguments to the builder to change its
+        behavior.
+        """
+        raise NotImplementedError
+
+
+class InstantiateAndSave(Builder):
+
+    def __call__(self, fixtures, klass, params, **kwargs):
+        """Save a fixture instance.
+
+        If it's a SQLAlchemy model, it will be added to the session and
+        the session will be committed.
+
+        Otherwise, a :meth:`save` method will be run if the instance has
+        one. If it does not have one, nothing will happen.
+
+        Before and after the process, the :func:`before_save` and
+        :func:`after_save` hook are run.
+
+        """
+        session = kwargs.get('session')
+        save = kwargs.get('save')
+
+        instance = self.instantiate(klass, params)
+        if save:
+            self.save(instance, fixtures, session)
+        return instance
+
+    def instantiate(self, klass, params):
+        """Return instantiated instance."""
+        try:
+            return klass(**params)
+        except TypeError as exc:
+            raise TypeError("Error while trying to build %r "
+                            "with %r: %s" % (klass, params, exc))
+
+    def save(self, instance, fixtures, session):
+        """Save instance."""
+        fixtures.get_hook("before_save")(instance)
+
+        if session and is_sqlalchemy_model(instance):
+            session.add(instance)
+            session.commit()
+
+        else:
+            getattr(instance, "save", lambda: None)()
+
+        fixtures.get_hook("after_save")(instance)
+
+
+class DeleteAndCommit(Builder):
+
+    def __call__(self, fixtures, instance, **kwargs):
+        session = kwargs.get('session')
+        commit = kwargs.get('commit')
+
+        fixtures.get_hook("before_uninstall")()
+        try:
+            if commit:
+                self.delete(instance, session)
+            else:
+                try:
+                    getattr(instance, "delete_instance")()
+                except AttributeError:
+                    getattr(instance, "delete", lambda: None)()
+
+        except Exception as exc:
+            fixtures.get_hook("after_uninstall")(exc)
+            raise
+
+        else:
+            fixtures.get_hook("after_uninstall")(None)
+
+    def delete(self, instance, session):
+        """Delete instance."""
+        if session and is_sqlalchemy_model(instance):
+            session.delete(instance)
+            session.commit()

charlatan/fixture.py

@@ -7,7 +7,7 @@
 from charlatan.utils import safe_iteritems
 
 CAN_BE_INHERITED = frozenset(
-    ["model_name", "fields", "post_creation", "depend_on"])
+    ["model_name", "models_package", "fields", "post_creation", "depend_on"])
 
 
 def get_class(module, klass):
@@ -37,7 +37,6 @@ def __init__(self, *args, **kwargs):
 
     def inherit_from_parent(self):
         """Inherit the attributes from parent, modifying itself."""
-
         if self._has_inherited_from_parent or not self.inherit_from:
             # Nothing to do
             return
@@ -47,9 +46,7 @@ def inherit_from_parent(self):
 
     def get_parent_values(self):
         """Return parent values."""
-
-        parent, _ = self.fixture_manager.fixture_collection.get(
-            self.inherit_from)
+        parent, _ = self.fixture_manager.collection.get(self.inherit_from)
         # Recursive to make sure everything is updated.
         parent.inherit_from_parent()
 
@@ -84,19 +81,23 @@ def __init__(self, key, fixture_manager,
                  model=None, fields=None,
                  inherit_from=None,
                  post_creation=None, id_=None,
+                 models_package='',
                  depend_on=frozenset()):
         """Create a Fixture object.
 
         :param str model: model used to instantiate the fixture, e.g.
             "yourlib.toaster:Toaster". If empty, the fields will be used as is.
+        :param str models_package: default models package for relative imports
         :param dict fields: args to be provided when instantiating the fixture
-        :param fixture_manager: FixtureManager creating the fixture
+        :param fixture_manager: FixturesManager creating the fixture
         :param dict post_creation: assignment to be done after instantiation
         :param str inherit_from: model to inherit from
         :param list depend_on: A list of relationships to depend on
 
-        """
+        .. versionadded:: 0.4.0
+            ``models_package`` argument added.
 
+        """
         super(Fixture, self).__init__()
 
         if id_ and fields:
@@ -111,21 +112,29 @@ def __init__(self, key, fixture_manager,
 
         # Stuff that can be inherited.
         self.model_name = model
+        self.models_package = models_package
         self.fields = fields or {}
         self.post_creation = post_creation or {}
         self.depend_on = depend_on
 
     def __repr__(self):
         return "<Fixture '%s'>" % self.key
 
-    def get_instance(self, path=None, fields=None):
+    def get_instance(self, path=None, overrides=None, builder=None):
         """Instantiate the fixture using the model and return the instance.
 
         :param str path: remaining path to return
-        :param dict fields: overriding fields
+        :param dict overrides: overriding fields
+        :param func builder: function that is used to get the fixture
+
+        .. deprecated:: 0.4.0
+            ``fields`` argument renamed ``overrides``.
+
+        .. versionadded:: 0.4.0
+            ``builder`` argument added.
 
         .. deprecated:: 0.3.7
-            ``include_relationships`` argument was removed.
+            ``include_relationships`` argument removed.
 
         """
         self.inherit_from_parent()  # Does the modification in place.
@@ -141,27 +150,22 @@ def get_instance(self, path=None, fields=None):
         else:
             # We need to do a copy since we're modifying them.
             params = copy.deepcopy(self.fields)
-            if fields:
-                params.update(fields)
+            if overrides:
+                params.update(overrides)
 
             for key, value in safe_iteritems(params):
                 if callable(value):
                     params[key] = value()
 
-            # Get the class to instantiate
+            # Get the class
             object_class = self.get_class()
 
             # Does not return anything, does the modification in place (in
             # fields).
             self._process_relationships(params)
 
             if object_class:
-                try:
-                    instance = object_class(**params)
-                except TypeError as exc:
-                    raise TypeError("Error while trying to instantiate %r "
-                                    "with %r: %s" %
-                                    (object_class, params, exc))
+                instance = builder(self.fixture_manager, object_class, params)
             else:
                 # Return the fields as is. This allows to enter dicts
                 # and lists directly.
@@ -181,16 +185,13 @@ def get_instance(self, path=None, fields=None):
 
     def get_class(self):
         """Return class object for this instance."""
-
-        root_models_package = self.fixture_manager.models_package
-
         if not self.model_name:
             return
 
         # Relative path, e.g. ".toaster:Toaster"
         if ":" in self.model_name and self.model_name[0] == ".":
             module, klass = self.model_name.split(":")
-            module = root_models_package + module
+            module = self.models_package + module
             return get_class(module, klass)
 
         # Absolute import, e.g. "yourlib.toaster:Toaster"
@@ -201,15 +202,15 @@ def get_class(self):
         # Class alone, e.g. "Toaster".
         # Trying to import from e.g.  yourlib.toaster:Toaster
         module = "{models_package}.{model}".format(
-            models_package=root_models_package,
+            models_package=self.models_package,
             model=self.model_name.lower())
         klass = self.model_name
 
         try:
             return get_class(module, klass)
         except ImportError:
             # Then try to import from yourlib:Toaster
-            return get_class(root_models_package, klass)
+            return get_class(self.models_package, klass)
 
     @staticmethod
     def extract_rel_name(name):
@@ -296,7 +297,6 @@ def _process_relationships(self, fields):
 
     def get_relationship(self, name):
         """Get a relationship and its attribute if necessary."""
-
         # This function is needed so that this fixture can require other
         # fixtures. If a fixture requires another fixture, it
         # necessarily means that it needs to include other relationships

charlatan/fixture_collection.py

@@ -17,6 +17,7 @@ class FixtureCollection(Inheritable):
 
     def __init__(self, key, fixture_manager,
                  model=None,
+                 models_package=None,
                  fields=None,
                  post_creation=None,
                  inherit_from=None,
@@ -25,7 +26,6 @@ def __init__(self, key, fixture_manager,
         super(FixtureCollection, self).__init__()
         self.key = key
         self.fixture_manager = fixture_manager
-        self.fields = fields or {}
         self.fixtures = fixtures or self.container()
 
         self.key = key
@@ -34,8 +34,9 @@ def __init__(self, key, fixture_manager,
         self._has_updated_from_parent = False
 
         # Stuff that can be inherited.
-        self.model_name = model
         self.fields = fields or {}
+        self.model_name = model
+        self.models_package = models_package
         self.post_creation = post_creation or {}
         self.depend_on = depend_on
 
@@ -45,16 +46,34 @@ def __repr__(self):
     def __iter__(self):
         return self.iterator(self.fixtures)
 
-    def get_instance(self, path=None, fields=None):
+    def get_instance(self, path=None, overrides=None, builder=None):
+        """Get an instance.
+
+        :param str path:
+        :param dict overrides:
+        :param func builder:
+        """
         if not path or path in (DICT_FORMAT, LIST_FORMAT):
-            return self.get_all_instances(fields=fields, format=path)
+            return self.get_all_instances(overrides=overrides,
+                                          format=path,
+                                          builder=builder,
+                                          )
 
         # First get the fixture
         fixture, remaining_path = self.get(path)
         # Then we ask it to return an instance.
-        return fixture.get_instance(path=remaining_path, fields=fields)
+        return fixture.get_instance(path=remaining_path,
+                                    overrides=overrides,
+                                    builder=builder,
+                                    )
 
-    def get_all_instances(self, fields=None, format=None):
+    def get_all_instances(self, overrides=None, format=None, builder=None):
+        """Get all instance.
+
+        :param dict overrides:
+        :param str format:
+        :param func builder:
+        """
         if not format:
             format = self.default_format
 
@@ -66,7 +85,8 @@ def get_all_instances(self, fields=None, format=None):
             raise ValueError("Unknown format: %r" % format)
 
         for name, fixture in self:
-            instance = fixture.get_instance(fields=fields)
+            instance = fixture.get_instance(overrides=overrides,
+                                            builder=builder)
 
             if format == LIST_FORMAT:
                 returned.append(instance)