diff --git a/base_manifest_extension/README.rst b/base_manifest_extension/README.rst index 652284ae6..da70ea7ca 100644 --- a/base_manifest_extension/README.rst +++ b/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. diff --git a/base_manifest_extension/__openerp__.py b/base_manifest_extension/__openerp__.py index 40963db91..892c18bd7 100644 --- a/base_manifest_extension/__openerp__.py +++ b/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', diff --git a/base_manifest_extension/hooks.py b/base_manifest_extension/hooks.py index 688965140..2e0c4d12d 100644 --- a/base_manifest_extension/hooks.py +++ b/base_manifest_extension/hooks.py @@ -1,21 +1,36 @@ # -*- coding: utf-8 -*- # Copyright 2017 Therp BV +# 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 =\ diff --git a/base_manifest_extension/tests/__init__.py b/base_manifest_extension/tests/__init__.py index 9ee84a5af..663f751fb 100644 --- a/base_manifest_extension/tests/__init__.py +++ b/base_manifest_extension/tests/__init__.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2017 Therp BV +# 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 diff --git a/base_manifest_extension/tests/test_base_manifest_extension.py b/base_manifest_extension/tests/test_base_manifest_extension.py deleted file mode 100644 index dc7434680..000000000 --- a/base_manifest_extension/tests/test_base_manifest_extension.py +++ /dev/null @@ -1,32 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright 2017 Therp BV -# 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) diff --git a/base_manifest_extension/tests/test_hooks.py b/base_manifest_extension/tests/test_hooks.py new file mode 100644 index 000000000..3e06b8cab --- /dev/null +++ b/base_manifest_extension/tests/test_hooks.py @@ -0,0 +1,185 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 Therp BV +# 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))