[Elisa-commits] [MERGE] resource providers API and manager

[Alessandro Decina]

On Tue, Apr 8, 2008 at 5:53 PM, Benjamin Kampmann <benjamin at fluendo.com> wrote:
> The Resource Providers API is considered stable and the resource manager
> works relieable now. We should merge both into the main rest branch.
>
>  This patch contains the resource_manager for core, a test for it with a
> coverage of 100%, the resource_provider base component and a modified
> application to hold the resource_manager.

bb:tweak

+    def get(self, uri, context_model=None):
[snip]
+        return (None, defer.fail(NotImplementedError))

this shuld be return (None, defer.fail(NotImplementedError())).
The same goes for the other methods.


@@ -202,6 +204,7 @@
         self._service_manager = service_manager.ServiceManager()
         self._metadata_manager = metadata_manager.MetadataManager()
         self._media_manager = media_manager.MediaManager(self.metadata_manager)
+        self._resource_manager = resource_manager.ResourceManager()
         self._input_manager = input_manager.InputManager()
         self._player_registry = player_registry.PlayerRegistry()
         self._interface_controller = interface_controller.InterfaceController()
@@ -362,6 +365,9 @@
     def metadata_manager__get(self):
         return self._metadata_manager

+    def resource_manager__get(self):
+        return self._resource_manager

Most of our __get and __set methods don't make any sense. This
definitely doesn't.
It should just be self.resource_manager = resource_manager.ResourceManager().
This is more a comment to the general style than to the current patch,
i'm fine with this hunk being
committed as it is, we can change it later.


+    def unregister_component(self, component):
+        Manager.unregister_component(self, component)
+        components = [c for r, c in self._providers_uri_regexps]
+        index = components.index(component)
+        self._providers_uri_regexps.pop(index)

Please don't use one-letter identifiers.


+    def _get_resource_provider(self, uri):
+        """
+        Retrieve the ResourceProvider able to handle L{uri}.
+        If multiple ResourceProviders support it, the first found is returned.
+
+        @param uri: location that will be passed to the resource provider
+                    returned
+        @type uri:  L{elisa.core.media_uri.MediaUri}
+
+        @rtype:     L{elisa.base_components.resource_provider.ResourceProvider}
+        """
+        result = None
+        for regexp, provider in self._providers_uri_regexps:
+            match = regexp.match(unicode(uri))
+            if match != None:
+                result = provider
+                break
+
+        if result != None:
+            self.debug("Using %r resource_provider to access %s", result.name,
+                       uri)
+            return result
+        else:
+            raise ResourceProviderNotFound(uri)

This looks good except that "if variable is None" and "if variable is
not None" are generally
preferred as they are faster and easier to read.


+    def put(self, source_uri, container_uri):
+        """
+        Put one resource into another. Both resources are identified with URIs.
+
+        @param source_uri:    URI pointing to the resource that should be put
+                              into the other one
+        @type source_uri:     L{elisa.core.media_uri.MediaUri}
+        @param container_uri: URI pointing to the resource that should receive
+                              the resource
+        @type container_uri:  L{elisa.core.media_uri.MediaUri}
+
+        @return:              a deferred fired when the resource got put
+        @rtype:               L{elisa.core.utils.defer.Deferred}
+        """
+        return self._proxy("put", container_uri, source_uri, container_uri)

I'm not sure I understand how put() is supposed to work. What if I
have a list of models and I decide to
save one somewhere? I can't pass the model to put so I guess that in
put you have to get() it again and
then upload/save it?


=== added file 'elisa-core/elisa/core/tests/test_resource_provider.py'
[snip]

This file doesn't really test anything and there is nothing to test as
the base ResourceProvider is just an
interface. It shoud be removed.

Overall the change looks ok (it's really simple, which is good).