[IMP] core,*: check routes decorators

When overriding an existing controller route, developers can
easily c/p the route definition and call super() in the overridden method
when the route attributes are automatically deducted by odoo from the parent route.

Removing those redefined attributes simplifies the routes definition,
clearly highlighting what's changed by the override.
Also reduces unexpected behavior when modifying the base route without
noticing/considering the redefined attributes in a overridden route,
which overrides the changes made to the base route when the sub-module is installed.

This commit adds a test to catch routes attributes redefinition, and clean existing routes.

closes odoo/odoo#108512

Related: odoo/enterprise#35176
Signed-off-by: Victor Feyens (vfe) <vfe@odoo.com>

addons/auth_signup/controllers/main.py

@@ -162,7 +162,7 @@ class AuthSignupHome(Home):
             raise SignupError(_('Authentication Failed.'))
 
 class AuthBaseSetup(BaseSetup):
-    @http.route('/base_setup/data', type='json', auth='user')
+    @http.route()
     def base_setup_data(self, **kwargs):
         res = super().base_setup_data(**kwargs)
         res.update({'resend_invitation': True})

addons/http_routing/controllers/main.py

@@ -20,6 +20,6 @@ class Routing(Home):
 
 class SessionWebsite(Session):
 
-    @http.route('/web/session/logout', type='http', auth="none", website=True, multilang=False, sitemap=False)
+    @http.route('/web/session/logout', website=True, multilang=False, sitemap=False)
     def logout(self, redirect='/web'):
         return super().logout(redirect=redirect)

addons/hw_posbox_homepage/controllers/main.py

@@ -97,7 +97,7 @@ class IoTboxHomepage(Home):
             'system': platform.system(),
             }
 
-    @http.route('/', type='http', auth='none')
+    @http.route()
     def index(self):
         wifi = Path.home() / 'wifi_network.txt'
         remote_server = Path.home() / 'odoo-remote-server.conf'

addons/sale_timesheet/controllers/portal.py

@@ -133,6 +133,6 @@ class SaleTimesheetCustomerPortal(TimesheetCustomerPortal):
                 })
         return values
 
-    @http.route(['/my/timesheets', '/my/timesheets/page/<int:page>'], type='http', auth="user", website=True)
-    def portal_my_timesheets(self, page=1, sortby=None, filterby=None, search=None, search_in='all', groupby='sol', **kw):
-        return super().portal_my_timesheets(page, sortby, filterby, search, search_in, groupby, **kw)
+    @http.route()
+    def portal_my_timesheets(self, *args, groupby='sol', **kw):
+        return super().portal_my_timesheets(*args, groupby=groupby, **kw)

addons/web/controllers/export.py

@@ -510,7 +510,7 @@ class ExportFormat(object):
 class CSVExport(ExportFormat, http.Controller):
 
     @http.route('/web/export/csv', type='http', auth="user")
-    def index(self, data):
+    def web_export_csv(self, data):
         try:
             return self.base(data)
         except Exception as exc:
@@ -554,7 +554,7 @@ class CSVExport(ExportFormat, http.Controller):
 class ExcelExport(ExportFormat, http.Controller):
 
     @http.route('/web/export/xlsx', type='http', auth="user")
-    def index(self, data):
+    def web_export_xlsx(self, data):
         try:
             return self.base(data)
         except Exception as exc:

addons/website/controllers/main.py

@@ -74,7 +74,7 @@ class QueryURL(object):
 
 class Website(Home):
 
-    @http.route('/', type='http', auth="public", website=True, sitemap=True)
+    @http.route('/', auth="public", website=True, sitemap=True)
     def index(self, **kw):
         """ The goal of this controller is to make sure we don't serve a 404 as
         the website homepage. As this is the website entry point, serving a 404

addons/website_crm_partner_assign/controllers/main.py

@@ -308,7 +308,7 @@ class WebsiteCrmPartnerAssign(WebsitePartnerPage):
 
 
     # Do not use semantic controller due to sudo()
-    @http.route(['/partners/<partner_id>'], type='http', auth="public", website=True)
+    @http.route()
     def partners_detail(self, partner_id, **post):
         _, partner_id = unslug(partner_id)
         current_grade, current_country = None, None

addons/website_event_meet/controllers/community.py

@@ -28,8 +28,7 @@ class WebsiteEventMeetController(EventCommunityController):
     # MAIN PAGE
     # ------------------------------------------------------------
 
-    @http.route(["/event/<model('event.event'):event>/community"], type="http",
-                auth="public", website=True, sitemap=True)
+    @http.route()
     def community(self, event, page=1, lang=None, **kwargs):
         """Display the meeting rooms of the event on the frontend side.
 

addons/website_event_meet_quiz/controllers/community.py

@@ -3,14 +3,13 @@
 
 from werkzeug.exceptions import Forbidden
 
-from odoo import http
 from odoo.addons.website_event.controllers.community import EventCommunityController
-from odoo.http import request
+from odoo.http import request, route
 
 
 class WebsiteEventTrackQuizMeetController(EventCommunityController):
 
-    @http.route(['/event/<model("event.event"):event>/community'], type='http', auth="public", website=True, sitemap=False)
+    @route()
     def community(self, event, page=1, lang=None, **kwargs):
         # website_event_track_quiz
         values = self._get_community_leaderboard_render_values(event, kwargs.get('search'), page)

addons/website_event_track_quiz/controllers/community.py

@@ -27,8 +27,7 @@ class WebsiteEventTrackQuizCommunityController(EventCommunityController):
         values = self._get_community_leaderboard_render_values(event, None, None)
         return request.render('website_event_track_quiz.event_leaderboard', values)
 
-    @http.route('/event/<model("event.event"):event>/community',
-                type='http', auth="public", website=True, sitemap=False)
+    @http.route()
     def community(self, event, **kwargs):
         values = self._get_community_leaderboard_render_values(event, None, None)
         return request.render('website_event_track_quiz.event_leaderboard', values)

addons/website_livechat/controllers/main.py

@@ -59,7 +59,7 @@ class WebsiteLivechat(LivechatController):
         }
         return request.render("website_livechat.channel_page", values)
 
-    @http.route('/im_livechat/get_session', type="json", auth='public', cors="*")
+    @http.route()
     def get_session(self, channel_id, anonymous_name, previous_operator_id=None, chatbot_script_id=None, persisted=True, **kwargs):
         """ Override to use visitor name instead of 'Visitor' whenever a visitor start a livechat session. """
         visitor_sudo = request.env['website.visitor']._get_visitor_from_request()

addons/website_sale_digital/controllers/main.py

@@ -14,9 +14,7 @@ from odoo.addons.sale.controllers.portal import CustomerPortal
 class WebsiteSaleDigital(CustomerPortal):
     orders_page = '/my/orders'
 
-    @http.route([
-        '/my/orders/<int:order_id>',
-    ], type='http', auth='public', website=True)
+    @http.route()
     def portal_order_page(self, order_id=None, **post):
         response = super(WebsiteSaleDigital, self).portal_order_page(order_id=order_id, **post)
         if not 'sale_order' in response.qcontext:

addons/website_sale_loyalty/controllers/main.py

@@ -35,7 +35,7 @@ class WebsiteSale(main.WebsiteSale):
             order._auto_apply_rewards()
         return res
 
-    @http.route(['/shop/cart'], type='http', auth="public", website=True)
+    @http.route()
     def cart(self, **post):
         order = request.website.sale_get_order()
         if order:

addons/website_slides/controllers/mail.py

@@ -20,7 +20,7 @@ class SlidesPortalChatter(PortalChatter):
             return True
         return super()._portal_post_has_content(res_model, res_id, message, attachment_ids=attachment_ids, **kw)
 
-    @http.route(['/mail/chatter_post'], type='json', methods=['POST'], auth='public', website=True)
+    @http.route()
     def portal_chatter_post(self, res_model, res_id, message, **kw):
         result = super(SlidesPortalChatter, self).portal_chatter_post(res_model, res_id, message, **kw)
         if result and res_model == 'slide.channel':

addons/website_slides_survey/controllers/slides.py

@@ -42,7 +42,7 @@ class WebsiteSlidesSurvey(WebsiteSlides):
     # Overrides
     # ------------------------------------------------------------
 
-    @http.route(['/slides/add_slide'], type='json', auth='user', methods=['POST'], website=True)
+    @http.route()
     def create_slide(self, *args, **post):
         create_new_survey = post['slide_category'] == "certification" and post.get('survey') and not post['survey']['id']
         linked_survey_id = int(post.get('survey', {}).get('id') or 0)

odoo/addons/test_lint/tests/__init__.py

@@ -11,3 +11,4 @@ from . import test_onchange_domains
 from . import test_override_signatures
 from . import test_pofile
 from . import test_pylint
+from . import test_routes

odoo/addons/test_lint/tests/test_routes.py

@@ -0,0 +1,48 @@
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
+import logging
+from pprint import pformat
+from unittest.mock import patch
+
+from odoo import http
+from odoo.tests import tagged, TransactionCase
+
+_logger = logging.getLogger(__name__)
+
+
+@tagged('post_install', '-at_install')
+class RoutesLinter(TransactionCase):
+
+    def test_routes_definition(self):
+        """Forbid redefinition of same-value attributes in an inherited route.
+
+        Makes it easier to know what an inherited route really modifies and
+        investigate unexpected behavior.
+        """
+        _check_and_complete_route_definition = http._check_and_complete_route_definition
+
+        def extended_check(controller_cls, submethod, merged_routing):
+            if 'type' in merged_routing:
+                # merged_routing contains non default 'type' value
+                # => current method is an inherited route.
+                useless_overrides = {
+                    key: value
+                    for key, value in submethod.original_routing.items()
+                    if key not in ('routes', 'type')
+                    if merged_routing.get(key) == value
+                }
+                if useless_overrides:
+                    _logger.warning(
+                        "The endpoint %s is duplicating the existing routing configuration : %s",
+                        f'{controller_cls.__module__}.{controller_cls.__name__}.{submethod.__name__}',
+                        pformat(useless_overrides),
+                    )
+
+            _check_and_complete_route_definition(controller_cls, submethod, merged_routing)
+
+        installed_modules = set(self.env['ir.module.module'].search([
+            ('state', '=', 'installed'),
+        ]).mapped('name'))
+        with patch('odoo.http._check_and_complete_route_definition', extended_check):
+            for _ in http._generate_routing_rules(installed_modules, nodb_only=False):
+                pass

odoo/http.py

@@ -797,13 +797,7 @@ def _generate_routing_rules(modules, nodb_only, converters=None):
                     _logger.warning("The endpoint %s is not decorated by @route(), decorating it myself.", f'{cls.__module__}.{cls.__name__}.{method_name}')
                     submethod = route()(submethod)
 
-                # Ensure "type" is defined on each method's own routing,
-                # also ensure overrides don't change the routing type.
-                default_type = submethod.original_routing.get('type', 'http')
-                routing_type = merged_routing.setdefault('type', default_type)
-                if submethod.original_routing.get('type') not in (None, routing_type):
-                    _logger.warning("The endpoint %s changes the route type, using the original type: %r.", f'{cls.__module__}.{cls.__name__}.{method_name}', routing_type)
-                submethod.original_routing['type'] = routing_type
+                _check_and_complete_route_definition(cls, submethod, merged_routing)
 
                 merged_routing.update(submethod.original_routing)
 
@@ -827,6 +821,24 @@ def _generate_routing_rules(modules, nodb_only, converters=None):
                 yield (url, endpoint)
 
 
+def _check_and_complete_route_definition(controller_cls, submethod, merged_routing):
+    """Verify and complete the route definition.
+
+    * Ensure 'type' is defined on each method's own routing.
+    * also ensure overrides don't change the routing type.
+
+    :param submethod: route method
+    :param dict merged_routing: accumulated routing values (defaults + submethod ancestor methods)
+    """
+    default_type = submethod.original_routing.get('type', 'http')
+    routing_type = merged_routing.setdefault('type', default_type)
+    if submethod.original_routing.get('type') not in (None, routing_type):
+        _logger.warning(
+            "The endpoint %s changes the route type, using the original type: %r.",
+            f'{controller_cls.__module__}.{controller_cls.__name__}.{submethod.__name__}',
+            routing_type)
+    submethod.original_routing['type'] = routing_type
+
 # =========================================================
 # Session
 # =========================================================