Hi - I've not made this class pluggable no, but if you can find a way to handle the situation you've described by updating the existing class - source code here - via a PR (or just suggestions) I'll be happy to try to include it.

Hi Andy,

Thanks for your reply.

What I usually do when I need this functionality is to create a CookieService og CookieProvider like you have.

It will have to methods, SetCookie and GetCookie (and CookieExists if needed).

The SetCookie method will set the cookie on the response and set the value in the HttpContext.Current.Items which exists only in the current request.

The GetCookie will then start by looking in the HttpContext.Current.Items and if no value with the specified key exists it looks for the cookie.

This way you can set a cookie in a request and get the "cookie" value in the same request.

I don't know if this is the preferred way by default in the plugin, but if the HttpContextCookieProvider was pluggable by registering a new implementation of the interface in the IOC container, I could do this in my specific solution.

It sounds to me that what you've described should likely be the default behaviour, so I'll take a look at implementing that when I get a bit of time. Or if you fancy PRing it, I'll be happy to review and include.

Andy

Hi Andy, I have created an example of a cookie provider. But with this users are forced to use this class to set cookies (so that the value is also set in the HttpContext.Current.Items with the right key), before they could set the cookie in the javascript or in whatever way they find best.

Maybe I think it's better if the HttpContextCookieProvider class and the ICookieProvider interface was registered in the IOC container, so users could simply register they own implementation of the interface?

public class HttpContextCookieProvider : ICookieProvider
{
    public bool CookieExists(string key)
    {
        return HttpContext.Current.Items.Contains($"personalisationGroups.cookie.{key}") || HttpContext.Current.Request.Cookies[key] != null;
    }

    public string GetCookieValue(string key)
    {
        if (HttpContext.Current.Items.Contains($"personalisationGroups.cookie.{key}"))
            return HttpContext.Current.Items[$"personalisationGroups.cookie.{key}"].ToString();

        return HttpContext.Current.Request.Cookies[key]?.Value;
    }

    public void SetCookie(HttpCookie cookie)
    {
        HttpContext.Current.Items[$"personalisationGroups.cookie.{cookie.Name}"] = cookie.Value;
        HttpContext.Current.Response.Cookies.Add(cookie);
    }
}

Thanks for this Jesper, this looks good to me.

On the IoC container point, of course you're fundamentally right and I agree that would be the best way to go.

The only concern I've got though is that this package also supports Umbraco V7 and I'm not sure if the IoC container support was there in core back in that version. I recall as a developer you could certainly introduce your own IoC container, but I'm not sure core shipped with one, and hence as a package developer you can't know what will be available.

So IIRC that's why I'm currently just "newing up" these classes in the code, and have a "poor man's DI" type of thing with the alternative constructor taking the interface purely to support unit testing.

Because of that I'm minded to just take and use your implementation - as it seems better to me, resolving the original issue you've raised - and just to be safe, introduce a config switch allowing people to opt-out of using the HttpContext.Items collection as well should they for some reason want to (and hence have the current behaviour).

Thoughts?

Andy

Hi Andy, I haven't thought about the V7 support and based on that, I agree with the thoughts you have and you are more than welcome to take my implementation.

Thanks Jesper... and apologies, I realise my comment to be "minded to just take and use your implementation" is a little presumptious on reading back!

But that's great, I'll get this introduced and a release out - hopefully over the weekend.

Andy

No worries, that was not how I perceived it, I'm happy to help out and thanks for taking the time to create a new release :-)

Thanks Andy, I have tested and it's working as expected.

is working on a reply...