[ADD] base_manifest_extension: rdepends_if_installed

* Add support for rdepends_if_installed key, which allows a module to specify
other installed modules that should list it as a dependency
* Add unit tests for new logic

base_manifest_extension/README.rst

@@ -10,9 +10,13 @@ This is a technical module that allows developers to make use of extra keys in
 module manifests. The following keys are available currently:
 
 * ``depends_if_installed`` - Your module will depend on modules listed here but
-  only if those modules are already installed. This is useful if your module is
-  supposed to override the behavior of a certain module (dependencies determine
-  load order) but does not need it to be installed.
+  only if those modules are already installed. This is useful if your module
+  needs to override the behavior of a certain module (dependencies determine
+  load order) but would also work without it.
+* ``rdepends_if_installed`` - The modules listed here will depend on your
+  module if they are already installed. This is useful if you want your module
+  to be higher in the inheritance chain than the target modules and do not want
+  to or cannot make changes to those modules.
 
 Usage
 =====
@@ -30,8 +34,6 @@ Add support for the following additional keys:
 * ``demo_if_installed``, ``data_if_installed``, ``qweb_if_installed`` - Dicts
   with module names as keys and lists of files as values. Used to load files
   only if some other module is installed.
-* ``rdepends_if_installed`` - Similar to ``depends_if_installed`` but
-  used to make another installed module depend on the current one.
 * ``_if_module`` - Used on models to load them only if the appropriate module
   is installed.
 

base_manifest_extension/__openerp__.py

@@ -6,6 +6,7 @@
 {
     'name': 'Module Manifest - Extra Options',
     'version': '9.0.1.0.0',
+    'website': 'https://github.com/OCA/server-tools',
     'author': 'Therp BV, LasLabs, Odoo Community Association (OCA)',
     'license': 'LGPL-3',
     'category': 'Hidden/Dependency',

base_manifest_extension/hooks.py

@@ -1,21 +1,36 @@
 # -*- coding: utf-8 -*-
 # Copyright 2017 Therp BV <http://therp.nl>
+# Copyright 2017 LasLabs Inc.
 # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html)
 
 import inspect
+from werkzeug.local import Local
 from openerp.sql_db import Cursor
 from openerp.modules import module
 from openerp.modules.graph import Graph
+
 original = module.load_information_from_description_file
+local = Local()
+local.rdepends_to_process = {}
 
 
 def load_information_from_description_file(module, mod_path=None):
     result = original(module, mod_path=mod_path)
+
     # add the keys you want to react on here
     if result.get('depends_if_installed'):
         cr = _get_cr()
         if cr:
             _handle_depends_if_installed(cr, result)
+    if result.get('rdepends_if_installed'):
+        cr = _get_cr()
+        if cr:
+            _handle_rdepends_if_installed(cr, result, module)
+
+    # Apply depends specified in other modules as rdepends
+    extra_depends = local.rdepends_to_process.get(module)
+    if extra_depends:
+        result['depends'] += extra_depends
 
     return result
 
@@ -23,17 +38,44 @@ def load_information_from_description_file(module, mod_path=None):
 def _handle_depends_if_installed(cr, manifest):
     if not manifest.get('depends_if_installed'):
         return
+
+    added_depends = manifest.pop('depends_if_installed')
+    added_depends = _installed_modules(cr, added_depends)
+
+    depends = manifest.setdefault('depends', [])
+    depends.extend(added_depends)
+
+
+def _handle_rdepends_if_installed(cr, manifest, current_module):
+    graph = _get_graph()
+    if not graph:
+        return
+
+    rdepends = manifest.pop('rdepends_if_installed')
+    rdepends = _installed_modules(cr, rdepends)
+
+    for rdepend in rdepends:
+        to_process = local.rdepends_to_process.get(rdepend, set([]))
+        local.rdepends_to_process[rdepend] = to_process | set([current_module])
+        # If rdepend already in graph, reload it so new depend is applied
+        if graph.get(rdepend):
+            del graph[rdepend]
+            graph.add_module(cr, rdepend)
+
+
+def _installed_modules(cr, modules):
+    if not modules:
+        return []
+
     cr.execute(
-        'select name from ir_module_module '
-        'where state in %s and name in %s',
+        'SELECT name FROM ir_module_module '
+        'WHERE state IN %s AND name IN %s',
         (
             tuple(['installed', 'to install', 'to upgrade']),
-            tuple(manifest['depends_if_installed']),
+            tuple(modules),
         ),
     )
-    manifest.pop('depends_if_installed')
-    depends = manifest.setdefault('depends', [])
-    depends.extend(module for module, in cr.fetchall())
+    return [module for module, in cr.fetchall()]
 
 
 def _get_cr():
@@ -62,15 +104,10 @@ def post_load_hook():
     cr = _get_cr()
     if not cr:
         return
-    cr.execute(
-        'select id from ir_module_module where name=%s and state in %s',
-        (
-            'base_manifest_extension',
-            tuple(['installed', 'to install', 'to upgrade']),
-        ),
-    )
+
     # do nothing if we're not installed
-    if not cr.rowcount:
+    installed = _installed_modules(cr, ['base_manifest_extension'])
+    if not installed:
         return
 
     module.load_information_from_description_file =\

base_manifest_extension/tests/__init__.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 # Copyright 2017 Therp BV <http://therp.nl>
+# Copyright 2017 LasLabs Inc.
 # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html)
 
-from . import test_base_manifest_extension
+from . import test_hooks

base_manifest_extension/tests/test_base_manifest_extension.py

@@ -1,32 +0,0 @@
-# -*- coding: utf-8 -*-
-# Copyright 2017 Therp BV <http://therp.nl>
-# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html)
-
-import os
-import tempfile
-from openerp.tests.common import TransactionCase
-from openerp.modules.module import load_information_from_description_file,\
-    get_module_path, MANIFEST
-
-
-class TestBaseManifestExtension(TransactionCase):
-    def test_base_manifest_extension(self):
-        # write a test manifest
-        module_path = tempfile.mkdtemp(dir=os.path.join(
-            get_module_path('base_manifest_extension'), 'static'
-        ))
-        with open(os.path.join(module_path, MANIFEST), 'w') as manifest:
-            manifest.write(repr({
-                'depends_if_installed': [
-                    'base_manifest_extension',
-                    'not installed',
-                ],
-            }))
-        # parse it
-        parsed = load_information_from_description_file(
-            # this name won't really be used, but avoids a warning
-            'base', mod_path=module_path,
-        )
-        self.assertIn('base_manifest_extension', parsed['depends'])
-        self.assertNotIn('not installed', parsed['depends'])
-        self.assertNotIn('depends_if_installed', parsed)

base_manifest_extension/tests/test_hooks.py

@@ -0,0 +1,185 @@
+# -*- coding: utf-8 -*-
+# Copyright 2017 Therp BV <http://therp.nl>
+# Copyright 2017 LasLabs Inc.
+# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html)
+
+from mock import Mock, patch
+import os
+import tempfile
+from openerp.modules.module import load_information_from_description_file,\
+    get_module_path, MANIFEST
+from openerp.tests.common import TransactionCase
+from ..hooks import _handle_rdepends_if_installed, _installed_modules
+
+MOCK_PATH = 'openerp.addons.base_manifest_extension.hooks'
+
+
+class TestHooks(TransactionCase):
+    def setUp(self):
+        super(TestHooks, self).setUp()
+
+        self.test_cr = self.env.cr
+        self.test_rdepends = [
+            'base',
+            'base_manifest_extension',
+            'not_installed',
+        ]
+        self.test_manifest = {
+            'rdepends_if_installed': self.test_rdepends,
+            'depends': [],
+        }
+        self.test_module_name = 'base_manifest_extension'
+        self.test_call = (
+            self.test_cr,
+            self.test_manifest,
+            self.test_module_name,
+        )
+
+    def test_base_manifest_extension(self):
+        # write a test manifest
+        module_path = tempfile.mkdtemp(dir=os.path.join(
+            get_module_path('base_manifest_extension'), 'static'
+        ))
+        with open(os.path.join(module_path, MANIFEST), 'w') as manifest:
+            manifest.write(repr({
+                'depends_if_installed': [
+                    'base_manifest_extension',
+                    'not installed',
+                ],
+            }))
+        # parse it
+        parsed = load_information_from_description_file(
+            # this name won't really be used, but avoids a warning
+            'base', mod_path=module_path,
+        )
+        self.assertIn('base_manifest_extension', parsed['depends'])
+        self.assertNotIn('not installed', parsed['depends'])
+        self.assertNotIn('depends_if_installed', parsed)
+
+    def test_installed_modules_correct_result(self):
+        """It should return only installed modules in list"""
+        result = _installed_modules(self.test_cr, self.test_rdepends)
+
+        expected = self.test_rdepends[:2]
+        self.assertEqual(result, expected)
+
+    def test_installed_modules_empty_starting_list(self):
+        """It should safely handle being passed an empty module list"""
+        result = _installed_modules(self.test_cr, [])
+
+        self.assertEqual(result, [])
+
+    @patch(MOCK_PATH + '._get_graph')
+    def test_handle_rdepends_if_installed_graph_call(self, graph_mock):
+        """It should call graph helper and return early if graph not found"""
+        graph_mock.return_value = None
+        graph_mock.reset_mock()
+        self.test_cr = Mock()
+        self.test_cr.reset_mock()
+        _handle_rdepends_if_installed(*self.test_call)
+
+        graph_mock.assert_called_once()
+        self.test_cr.assert_not_called()
+
+    @patch(MOCK_PATH + '._get_graph')
+    def test_handle_rdepends_if_installed_clean_manifest(self, graph_mock):
+        """It should remove rdepends key from manifest"""
+        _handle_rdepends_if_installed(*self.test_call)
+
+        self.assertEqual(self.test_manifest, {'depends': []})
+
+    @patch(MOCK_PATH + '.local.rdepends_to_process', new_callable=dict)
+    @patch(MOCK_PATH + '._get_graph')
+    def test_handle_rdepends_if_installed_list(self, graph_mock, dict_mock):
+        """It should correctly add all installed rdepends to processing dict"""
+        _handle_rdepends_if_installed(*self.test_call)
+
+        expected_result = {
+            'base': set([self.test_module_name]),
+            'base_manifest_extension': set([self.test_module_name]),
+        }
+        self.assertEqual(dict_mock, expected_result)
+
+    @patch(MOCK_PATH + '.local.rdepends_to_process', new_callable=dict)
+    @patch(MOCK_PATH + '._get_graph')
+    def test_handle_rdepends_if_installed_dupes(self, graph_mock, dict_mock):
+        """It should correctly handle multiple calls with same rdepends"""
+        for __ in range(2):
+            _handle_rdepends_if_installed(*self.test_call)
+            self.test_manifest['rdepends_if_installed'] = self.test_rdepends
+        test_module_name_2 = 'test_module_name_2'
+        _handle_rdepends_if_installed(
+            self.test_cr,
+            self.test_manifest,
+            test_module_name_2,
+        )
+
+        expected_set = set([self.test_module_name, test_module_name_2])
+        expected_result = {
+            'base': expected_set,
+            'base_manifest_extension': expected_set,
+        }
+        self.assertEqual(dict_mock, expected_result)
+
+    @patch(MOCK_PATH + '._get_graph')
+    def test_handle_rdepends_if_installed_graph_reload(self, graph_mock):
+        """It should reload installed rdepends already in module graph"""
+        class TestGraph(dict):
+            pass
+
+        test_graph = TestGraph(base='Test Value')
+        test_graph.add_module = Mock()
+        graph_mock.return_value = test_graph
+        _handle_rdepends_if_installed(*self.test_call)
+
+        self.assertEqual(test_graph, {})
+        test_graph.add_module.assert_called_once_with(self.cr, 'base')
+
+    @patch(MOCK_PATH + '._handle_rdepends_if_installed')
+    @patch(MOCK_PATH + '._get_cr')
+    @patch(MOCK_PATH + '.original')
+    def test_load_information_from_description_file_rdepends_key(
+        self, super_mock, cr_mock, helper_mock
+    ):
+        """It should correctly call rdepends helper if key present"""
+        super_mock.return_value = self.test_manifest
+        cr_mock.return_value = self.cr
+        helper_mock.reset_mock()
+        load_information_from_description_file(self.test_module_name)
+
+        helper_mock.assert_called_once_with(*self.test_call)
+
+    @patch(MOCK_PATH + '._handle_rdepends_if_installed')
+    @patch(MOCK_PATH + '._get_cr')
+    @patch(MOCK_PATH + '.original')
+    def test_load_information_from_description_file_no_rdepends_key(
+        self, super_mock, cr_mock, helper_mock
+    ):
+        """It should not call rdepends helper if key not present"""
+        del self.test_manifest['rdepends_if_installed']
+        super_mock.return_value = self.test_manifest
+        cr_mock.return_value = self.cr
+        helper_mock.reset_mock()
+        load_information_from_description_file(self.test_module_name)
+
+        helper_mock.assert_not_called()
+
+    @patch(MOCK_PATH + '._get_cr')
+    @patch(MOCK_PATH + '.original')
+    def test_load_information_from_description_file_rdepends_to_process(
+        self, super_mock, cr_mock
+    ):
+        """It should correctly add pending rdepends to manifest"""
+        del self.test_manifest['rdepends_if_installed']
+        super_mock.return_value = self.test_manifest
+        cr_mock.return_value = self.cr
+        test_depends = set(['Test Depend 1', 'Test Depend 2'])
+        test_rdepend_dict = {
+            self.test_module_name: test_depends,
+            'Other Module': set(['Other Depend']),
+        }
+        dict_path = MOCK_PATH + '.local.rdepends_to_process'
+        with patch.dict(dict_path, test_rdepend_dict, clear=True):
+            load_information_from_description_file(self.test_module_name)
+
+        self.assertEqual(self.test_manifest['depends'], list(test_depends))