Edgewall Software

Opened 5 years ago

Closed 5 years ago

#12713 closed enhancement (fixed)

Update of callbacks dict in RequestDispatcher.dispatch makes it difficult to write test case

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.2.1
Component: web frontend Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:
API Changes:

Update of Request callbacks is done by set_default_callbacks method, rather than dispatch method.

Internal Changes:


For example I've written some test case like browser:trachacksplugin/trunk/trachacks/tests/web_ui.py@15127:109#L104, but when I try to use MockRequest the tests don't pass because the callbacks are overwritten: tags/trac-1.2/trac/web/main.py@:195-206#L184.

I can make the tests work with the following change:

  • trac/web/main.py

    diff --git a/trac/web/main.py b/trac/web/main.py
    index b0680ab5b..261debb27 100644
    a b class RequestDispatcher(Component):  
    192192        chrome = Chrome(self.env)
    194194        # Setup request callbacks for lazily-evaluated properties
    195         req.callbacks.update({
     195        default_callbacks = {
    196196            'authname': self.authenticate,
    197197            'chrome': chrome.prepare_request,
    198198            'form_token': self._get_form_token,
    class RequestDispatcher(Component):  
    203203            'tz': self._get_timezone,
    204204            'use_xsendfile': self._get_use_xsendfile,
    205205            'xsendfile_header': self._get_xsendfile_header,
    206         })
     206        }
     207        for key, value in default_callbacks.iteritems():
     208            req.callbacks.setdefault(key, value)
    208210        try:
    209211            # Select the component that should handle the request

Does anyone foresee any negative side effects?

Attachments (0)

Change History (5)

comment:1 by Ryan J Ollos, 5 years ago

Changes in log:rjollos.git:t12713_req_callbacks. However, changes to test cases are just workarounds to get test to pass. If we adopt the change I think we should not use MockRequest in trac.web.tests.main, rather use _make_environ and _make_req as in trac.web.tests.api.

comment:2 by Ryan J Ollos, 5 years ago

Possible alternate solution:

  • trac/web/main.py

    diff --git a/trac/web/main.py b/trac/web/main.py
    index b0680ab5b..430403e71 100644
    a b class RequestDispatcher(Component):  
    191191        self.log.debug('Dispatching %r', req)
    192192        chrome = Chrome(self.env)
    194         # Setup request callbacks for lazily-evaluated properties
    195         req.callbacks.update({
    196             'authname': self.authenticate,
    197             'chrome': chrome.prepare_request,
    198             'form_token': self._get_form_token,
    199             'lc_time': self._get_lc_time,
    200             'locale': self._get_locale,
    201             'perm': self._get_perm,
    202             'session': self._get_session,
    203             'tz': self._get_timezone,
    204             'use_xsendfile': self._get_use_xsendfile,
    205             'xsendfile_header': self._get_xsendfile_header,
    206         })
    208194        try:
    209195            # Select the component that should handle the request
    210196            chosen_handler = None
    class RequestDispatcher(Component):  
    318304    # Internal methods
     306    def set_default_callbacks(self, req):
     307        """Setup request callbacks for lazily-evaluated properties.
     308        """
     309        req.callbacks.update({
     310            'authname': self.authenticate,
     311            'chrome': Chrome(self.env).prepare_request,
     312            'form_token': self._get_form_token,
     313            'lc_time': self._get_lc_time,
     314            'locale': self._get_locale,
     315            'perm': self._get_perm,
     316            'session': self._get_session,
     317            'tz': self._get_timezone,
     318            'use_xsendfile': self._get_use_xsendfile,
     319            'xsendfile_header': self._get_xsendfile_header,
     320        })
    320322    @lazy
    321323    def _request_handlers(self):
    322324        return dict((handler.__class__.__name__, handler)
    def _dispatch_request(req, env, env_error):  
    615617    try:
    616618        if not env and env_error:
    617619            raise HTTPInternalError(env_error)
     620        dispatcher = RequestDispatcher(env)
    618621        try:
    619             dispatcher = RequestDispatcher(env)
     622            dispatcher.set_default_callbacks(req)
    620623            dispatcher.dispatch(req)
    621624        except RequestDone as req_done:
    622625            resp = req_done.iterable

comment:3 by Jun Omae, 5 years ago

The changes in comment:2 sound good. It might be able to change the callbacks by inheriting the dispatcher for other purposes.

comment:4 by Ryan J Ollos, 5 years ago

API Changes: modified (diff)

Thanks, I also prefer the comment:2 change.

comment:5 by Ryan J Ollos, 5 years ago

Resolution: fixed
Status: assignedclosed

Committed to 1.2-stable in [15632:15633], merged to trunk in [15634]

Modify Ticket

Change Properties
Set your email in Preferences
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment

E-mail address and name can be saved in the Preferences .
Note: See TracTickets for help on using tickets.