Open atul898 opened 11 years ago
Hey
Yeah I can add "callback" querystring param to ignored list. Will do later today
cheers
Yes that will be great, I ended up doing something similar on my local copy. Added a ExcludeJsonpCallbackFromCacheKey property to the CacheOutput attribute. Great!
Hey it's updated - (we will now exclude "callback" from the cache key) and also pushed to nuget - http://nuget.org/packages/strathweb.cacheoutput
Hi Filip,
I have noticed a bug in the jsonp related cache implementation. but I have not been able to figure out where exactly is the problem. I thought I will share this with you, and see if you have any thoughts on this.
Here is what i see. When we have caching enabled for a jsonp call, I see on my browser's debug window that the jsonp call works the first time (because caching has not happened yet), but when I make the call again, it works but not perfectly. (actually it fails, the data comes back, but in a wrong wrapper) On the browser's debug windows I see that the call was made something like this http://localhost:60074/api/abcController?somedata=abcdef&callback=jQuery18306711026045959443_1361901018877&_=1361901018989 where jQuery183067110260459594431361901018877&=1361901018989 was auto generated by jQuery's getJSON method call I made in my javascript call. When the call works (the very first call with caching enabled) the response is jQuery18306711026045959443_1361901018877(myJSONresponse) which is perfect. (Do notice here that for jQuery's getJSON method this call was a success because we got the response wrapped in "jQuery18306711026045959443_1361901018877" the same string that was part of the original call.) All this is good.
But when the second or further calls are made (with caching on) the response is something like this (though the wrapper changes everytime) jQuery183032068624696694314_1361901471618(myJSONresponse) I mean to say that the jQuery wrapper string has changed to something else/new, hence the call is marked as failed, and I see "Uncaught ReferenceError: jQuery183032068624696694314_1361901471618 is not defined"
Any thoughts?
Regards Atul
Found the root cause of the problem above. In WebApiCache you cache the jsonpWrapper too, while you should be caching just the value contained in that wrapper.
Here is my local fix for your reference.
I have fixed this bug in my local copy.
public override void OnActionExecuting(HttpActionContext actionContext)
{
if (actionContext == null) throw new ArgumentNullException("actionContext");
if (!_isCachingAllowed(actionContext, AnonymousOnly)) return;
var config = actionContext.Request.GetConfiguration();
EnsureCacheTimeQuery();
EnsureCache(config, actionContext.Request);
_responseMediaType = GetExpectedMediaType(config, actionContext);
var cachekey = MakeCachekey(actionContext, _responseMediaType, ExcludeQueryStringFromCacheKey);
if (!WebApiCache.Contains(cachekey)) return;
if (actionContext.Request.Headers.IfNoneMatch != null)
{
var etag = WebApiCache.Get(cachekey + Constants.EtagKey) as EntityTagHeaderValue;
if (etag != null)
{
if (actionContext.Request.Headers.IfNoneMatch.Any(x => x.Tag == etag.Tag))
{
var time = CacheTimeQuery.Execute(DateTime.Now);
var quickResponse = actionContext.Request.CreateResponse(HttpStatusCode.NotModified);
ApplyCacheHeaders(quickResponse, time);
actionContext.Response = quickResponse;
return;
}
}
}
var val = WebApiCache.Get(cachekey) as string;
if (val == null) return;
var jsonpWrapper = GetJsonpCallback(actionContext.Request);
if (jsonpWrapper != null)
{
val = string.Format("{0}({1})", jsonpWrapper, val);
}
var contenttype = (MediaTypeHeaderValue)WebApiCache.Get(cachekey + Constants.ContentTypeKey) ??
new MediaTypeHeaderValue(cachekey.Split(':')[1]);
actionContext.Response = actionContext.Request.CreateResponse();
actionContext.Response.Content = new StringContent(val);
actionContext.Response.Content.Headers.ContentType = contenttype;
var responseEtag = WebApiCache.Get(cachekey + Constants.EtagKey) as EntityTagHeaderValue;
if (responseEtag != null) SetEtag(actionContext.Response, responseEtag.Tag);
var cacheTime = CacheTimeQuery.Execute(DateTime.Now);
ApplyCacheHeaders(actionContext.Response, cacheTime);
}
public override void OnActionExecuted(HttpActionExecutedContext actionExecutedContext)
{
if (!_isCachingAllowed(actionExecutedContext.ActionContext, AnonymousOnly)) return;
var cacheTime = CacheTimeQuery.Execute(DateTime.Now);
var cachekey = MakeCachekey(actionExecutedContext.ActionContext, _responseMediaType, ExcludeQueryStringFromCacheKey);
if (!string.IsNullOrWhiteSpace(cachekey) && !(WebApiCache.Contains(cachekey)))
{
SetEtag(actionExecutedContext.Response, Guid.NewGuid().ToString());
actionExecutedContext.Response.Content.ReadAsStringAsync().ContinueWith(t =>
{
var baseKey = actionExecutedContext.Request.GetConfiguration().CacheOutputConfiguration().MakeBaseCachekey(actionExecutedContext.ActionContext.ControllerContext.ControllerDescriptor.ControllerName, actionExecutedContext.ActionContext.ActionDescriptor.ActionName);
WebApiCache.Add(baseKey, string.Empty, cacheTime.AbsoluteExpiration);
var jsonpWrapper = GetJsonpCallback(actionExecutedContext.Request);
var realResult = string.Empty;
if (jsonpWrapper != null)
{
//Cache the value contained in jsonp wrapper
var jsonpWrapperLength = jsonpWrapper.Length;
realResult = t.Result.Substring(jsonpWrapperLength + 1, t.Result.Length - jsonpWrapperLength - 2);
}
else
{
realResult = t.Result;
}
WebApiCache.Add(cachekey, realResult, cacheTime.AbsoluteExpiration, baseKey);
WebApiCache.Add(cachekey + Constants.ContentTypeKey,
actionExecutedContext.Response.Content.Headers.ContentType,
cacheTime.AbsoluteExpiration, baseKey);
WebApiCache.Add(cachekey + Constants.EtagKey,
actionExecutedContext.Response.Headers.ETag,
cacheTime.AbsoluteExpiration, baseKey);
});
}
//if (!_isCachingAllowed(actionExecutedContext.ActionContext, AnonymousOnly)) return;
ApplyCacheHeaders(actionExecutedContext.ActionContext.Response, cacheTime);
}
Hi, I have the same problem. It would be nice to get this fix pushed to Nuget. I'll get a local copy for now until you can get it published to nuget.
Thanks for a great cache tool.
Is this bug fixed? I am using jsonp and something is not right
I had hacked Filip's source code to support jsonp properly. I suppose I had put (the fix as) a comment on the issue list. Try that.
On Tue, Nov 26, 2013 at 9:36 AM, lbusuioc notifications@github.com wrote:
Is this bug fixed? I am using jsonp and something is not right
— Reply to this email directly or view it on GitHubhttps://github.com/filipw/AspNetWebApi-OutputCache/issues/8#issuecomment-29307885 .
I did that and it worked perfectly.
Do you use this in production? Any issues? I noticed someone reported the application crashed.
Yeah it is in production and I haven't seen any problems yet. Though you could look at using CORS that enables you to use JSON calls instead of JSOP on your API methods. I haven't used that so I don't know if it will work together with this package.
Any update on whether this will be corrected? I'd love to use this but would need it to support JSONP for now - we're not ready to switch to CORS yet.
This is very helpful.
I do have a question/issue to mention though, How do you cache/handle jsonp calls? Since for jsonp calls the query string tends to be similar to http://localhost:60074/api/order?data=022eb4b7dd5e33ddbfcc1dab63bd203f&callback=jQuery18306690503899008_879798
The 'callback' variable's value will be different for each call, though the input variable 'data' may be contain same value for various calls. Is there a way to ignore 'callback' variable's value while caching?