kepazon / mvc-mini-profiler

Automatically exported from code.google.com/p/mvc-mini-profiler
0 stars 0 forks source link

SetViewed not working correctly with HttpRuntimeCacheStorage #131

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. In Sample.Mvc set MiniProfiler.Settings.Storage to HttpRuntimeCacheStorage
2. Run Sample.Mvc
3. Refresh the page a few times

=> Each request shows the timings from the all the previous requests as well.

Quick fix: Add 
    WebRequestProfilerProvider.Settings.UserProvider = new IpAddressIdentity();
to InitProfilerSettings()

This looks like it is caused by the code being in transition between two 
approaches, so I am not doing a pull request as I am not sure what the correct 
answer is.

The problem is caused by this code in MiniProfilerHandler:

            var provider = WebRequestProfilerProvider.Settings.UserProvider;
            string user = null;
            if (provider != null)
            {
                user = provider.GetUser(context.Request);
            }

            MiniProfiler.Settings.Storage.SetViewed(user, id);

When no UserProvider is set, the user is null. With HttpRuntimeCacheStorage 
this causes the SetViewed to not set anything viewed. In contrast, this works 
fine with the SqlServerStorage provider as that completely ignores the user 
that is passed in.

I suspect SqlServerStorage is supposed to take the user into account, but that 
has not been implemented yet. 

In terms of solutions, I think the answer is either;
- Make sure WebRequestProfilerProvider.Settings.UserProvider has a default
or
- In the above code, use profiler.User instead of getting the provider again. 

I would do a pull request, but I am unsure if using profiler.User or using the 
UserProvider is the right approach; The code seems to be in transition between 
the two approaches?

Thanks for building this tool guys, it is incredible!

Original issue reported on code.google.com by franslyt...@gmail.com on 28 Jan 2012 at 4:24

GoogleCodeExporter commented 9 years ago
is this still the case, I did muck around with that code lately 

Original comment by sam.saff...@gmail.com on 8 Feb 2012 at 2:58

GoogleCodeExporter commented 9 years ago
Sorry Sam, still the same... 
I am happy to do a pull request if you can tell me if the code is supposed to 
use WebRequestProfilerProvider.Settings.UserProvider by default now instead of 
profiler.User? 
If so, I'll just change that property to always return an IpAddressIdentity() 
if it hasn't been set to anything else. While I am at it, should I change all 
other references to profiler.User? I could change profiler.User to return the 
result of a call to WebRequestProfilerProvider.Settings.UserProvider?
The problem seems to stem from there being two separate ways of determining the 
user and one can return a value while the other returns null.

Original comment by franslyt...@gmail.com on 11 Feb 2012 at 2:45

GoogleCodeExporter commented 9 years ago
WebRequestProfilerProvider.Settings.UserProvider is the only valid way of 
determining the user, I nuked the one hanging off settings. I also cleaned up 
the little null trickery that was causing the mess ... can you confirm this is 
working now? 

Original comment by sam.saff...@gmail.com on 13 Feb 2012 at 5:44

GoogleCodeExporter commented 9 years ago
Yep, all working now, thanks Sam!

Original comment by franslyt...@gmail.com on 13 Feb 2012 at 2:51