Root app middleware receives wrong app context

[popravich]

Long story short

When handling request to sub-application root application's middlewares see wrong
request.app instance — sub-app instead of the one they attached to.

This PR demonstrates it (adds failing test) but has no fix yet.

[asvetlov]

Is it related to #2454 ?

[popravich]

not sure

[popravich]

request.app property always returns the last app in match_info.apps list.
We need to fix this.

[popravich]

So, the fix is as following:

• make request.app normal property, not reify'ed attribute;
• make request.app return match_info.current_app
• add match_info.current_app property which, by default, returns last app (same as request.app did).
• add a context manager to match_info that updates current_app;
• update application's _prepare_middleware method to append one extra middleware
  _fix_request_current_app that switches current application in match info and, as a result, in request.

Whew... profit!

Guys, your thoughts?

[codecov-io]

Codecov Report

Merging #2550 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2550      +/-   ##
==========================================
+ Coverage   97.55%   97.57%   +0.01%     
==========================================
  Files          40       40              
  Lines        8057     8078      +21     
  Branches     1409     1411       +2     
==========================================
+ Hits         7860     7882      +22     
+ Misses         75       74       -1     
  Partials      122      122

Impacted Files	Coverage Δ
aiohttp/web_urldispatcher.py	99.15% <100%> (+0.01%)	⬆️
aiohttp/web_middlewares.py	100% <100%> (ø)	⬆️
aiohttp/web.py	97.87% <100%> (+0.38%)	⬆️
aiohttp/web_request.py	99.68% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bbefc20...91cc566. Read the comment docs.

[asvetlov]

Adding a new middleware is not ideal maybe but I can live with it -- an extra middleware is better than broken functionality.
We can invite a better fix later anyway.

[SoumenDass]

This change has broken existing functionality e.g. earlier within a view in a sub-application, I could always do request.app and get the sub-application instance. Now it always returns the main application instance which basically defeats the purpose of sub-application. E.g. if I have a routes defined in the sub-application, earlier they used to work fine via request.app.router['<route_name>']. Now since request.app always returns the main app, it is not aware of the subapplication route names and errors out:

    rpt_url = request.app.router['getreport'].url_for(rpt_id=rpt_id)
  File "/home/sdass/apivenv/lib/python3.6/site-packages/aiohttp/web_urldispatcher.py", line 851, in __getitem__
    return self._named_resources[name]
KeyError: 'getreport'

[popravich]

Hi, @SoumenDass ,
does your applications (main and sub) have any middlewares setup?

[SoumenDass]

All my middlewares are in the main app

[popravich]

ok, thanks, I will check your issue

[SoumenDass]

Thanks - for now, I've rolled back my deployment to 2.3.3 where this issue does not happen.

[popravich]

I've just updated the test and made a fix.
Indeed, if sub-app has no middleware it was accessing main app instance.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new