Coming from #2286971-83: Remove dependency of current_user on request and authentication manager
The
PathBasedBreadcrumbBuilder
is using theAccessAwareRouter
to find matching routes as it is building the breadcrumb off of the current path. Unfortunately, the$request
object passed in is incomplete (no route parameters) resulting in the exceptions as well as the auth failures.Another undesirable side effect of this is that authentication is re-run multiple times as the breadcrumb links are being built.
A way to fix this would be to changing thePathBasedBreadcrumbBuilder
to use a route matcher (e.g.NestedMatcher
) instead of a router to build its links.
Each time a breadcrumb is built, all route enhancers (including ParamConverters and EntityRouteEnhancers) are called for each link in the breadcrumb. This is not good for performance.
Using a request matcher directly would short circuit the calls to the route enhancers.
Comments
Comment #1
larowlanYep it was only supposed to be a temporary solution
Comment #2
almaudoh CreditAttribution: almaudoh commentedComment #3
dawehnerDo you have numbers to prove that? That kinda should be a first step I think.
In general I'm curious how we could avoid calling out to things, given that all that information are needed ...
Comment #4
almaudoh CreditAttribution: almaudoh commented@dawehner: don't have actual numbers yet because I don't yet have a working patch that doesn't call out to route enhancers. Still work in progress...
Comment #5
dawehnerIts not really obvious how would you build such a router without checking access in the first place?
Comment #6
Crell CreditAttribution: Crell at Palantir.net commentedYou'd have to make the assumption that a parent path of the current page is going to be accessible to the current user. If you make that assumption, you don't need to access check those parents and can skip that work. I don't know that's a safe assumption to make, though...