Cookie repository for self-hosted web service

Question

I use this in a self hosted web service, so I am not using a ton of libraries. I wanted a simple yet easy-to-use cookie repository. The reason the repository is static is that I wanted the ability to use the repository within Razor templates without having to use a reference object. I just find it simpler.

I feel it's pretty well-rounded in functionality as it stands right now, but a review is always nice.

using System;
using System.Collections.Generic;
using System.Linq;

namespace CookieRepository
{
    class Program
    {
        static void Main(string[] args)
        {
            CookieRepository<bool>.Add("myid", "Authenticated", true);

            // do something...


            // Check cookie again.
            if (CookieRepository<bool>.ValueEquals("myid", "Authenticated", true))
            {
                Console.WriteLine("Value was true, user is Authenticated.");
            }
            else
            {
                Console.WriteLine("Value was false, user is NOT Authenticated.");
            }

            // do something...


            // Update cookie.
            CookieRepository<bool>.Set("myid", "Authenticated", false);

            // do something...


            // Lets do another check.
            if (CookieRepository<bool>.ValueEquals("myid", "Authenticated", true))
            {
                Console.WriteLine("Value was true, user is Authenticated.");
            }
            else
            {
                Console.WriteLine("Value was false, user is NOT Authenticated.");
            }

            Console.WriteLine("Press any key to exit...");
            Console.ReadKey();
        }
    }

    public static class CookieRepository<T>
    {
        private static readonly HashSet<Cookie<T>> Cookies = new HashSet<Cookie<T>>();

        /// <summary>
        /// Used when you want to add a cookie and being sure that cookie by same Key does not already exist.
        /// </summary>
        /// <param name="SessionId">Session Id of owner</param>
        /// <param name="Key">Cookie known as</param>
        /// <param name="Value">Value of the cookie</param>
        /// <returns></returns>
        public static bool Add(string SessionId, string Key, T Value)
        {
            lock (Cookies)
            {
                if (Cookies.Any(x => x.SessionId == SessionId && x.Key == Key))
                {
                    return false;
                }
                else
                {
                    Cookies.Add(new Cookie<T>(SessionId, Key, Value));
                }

                return true;
            }
        }

        /// <summary>
        /// Adds or Replaces a cookie, used when ensurance that cookie by key does not already exist is of any importance.
        /// </summary>
        /// <param name="SessionId">Session Id of owner</param>
        /// <param name="Key">Cookie known as</param>
        /// <param name="Value">Value of the cookie</param>
        public static void Set(string SessionId, string Key, T Value)
        {
            lock (Cookies)
            {
                var Cookie = GetCookie(SessionId, Key);

                if (Cookie != null)
                {
                    Cookies.Remove(Cookie);
                }

                Cookies.Add(new Cookie<T>(SessionId, Key, Value));
            }
        }

        /// <summary>
        /// Removes Cookie by specific key.
        /// </summary>
        /// <param name="SessionId">Session Id of owner</param>
        /// <param name="Key">Cookie known as</param>
        public static void Remove(string SessionId, string Key)
        {
            lock (Cookies)
            {
                var Cookie = GetCookie(SessionId, Key);

                if (Cookie != null)
                {
                    Cookies.Remove(Cookie);
                }
            }
        }

        /// <summary>
        /// Removes all cookies for a specific Session. (On disconnect)
        /// </summary>
        /// <param name="SessionId">Session Id of owner</param>
        public static void RemoveAll(string SessionId)
        {
            lock (Cookies)
            {
                var SessionCookies = Cookies.Where(x => x.SessionId == SessionId).ToList();

                foreach (var Cookie in SessionCookies)
                {
                    Cookies.Remove(Cookie);
                }
            }
        }

        /// <summary>
        /// Returns ALL cookies that exist in cache by Key
        /// </summary>
        /// <param name="key">Cookie known as</param>
        /// <returns>Queryable Interface</returns>
        public static IEnumerable<Cookie<T>> GetAll(string Key)
        {
            lock (Cookies)
            {
                return Cookies.Where(x => x.Key == Key);
            }
        }

        /// <summary>
        /// Tries to get Value from an Cookie by Owner and Key
        /// </summary>
        /// <param name="SessionId"></param>
        /// <param name="Key">Cookie known as</param>
        /// <param name="value">Out Cookie</param>
        /// <returns>Whether any Cookie Value could be found</returns>
        public static bool TryGetValue(string SessionId, string Key, out T value)
        {
            value = GetValue(SessionId, Key);

            return value != null;
        }

        /// <summary>
        /// Returns Cookie value by Owner and Key
        /// </summary>
        /// <param name="sessionid">Session Id of owner</param>
        /// <param name="key">Cookie known as</param>
        /// <returns>Cookie Value</returns>
        private static T GetValue(string sessionid, string key)
        {
            lock (Cookies)
            {
                return Cookies.Where(x => x.SessionId == sessionid && x.Key == key).Select(x => x.Value).FirstOrDefault();
            }
        }

        /// <summary>
        /// Tries to get Cookie by Owner and Key
        /// </summary>
        /// <param name="SessionId">Session Id of owner</param>
        /// <param name="Key">Cookie known as</param>
        /// <param name="Cookie">Out Cookie</param>
        /// <returns>Whether any Cookie could could be found</returns>
        public static bool TryGetCookie(string SessionId, string Key, out Cookie<T> Cookie)
        {
            Cookie = GetCookie(SessionId, Key);

            return Cookie != null;
        }

        /// <summary>
        /// Returns Cookie by Owner and Key
        /// </summary>
        /// <param name="SessionId">Session Id of owner</param>
        /// <param name="Key">Cookie known as</param>
        /// <returns>Cookie Object</returns>
        private static Cookie<T> GetCookie(string SessionId, string Key)
        {
            lock (Cookies)
            {
                return Cookies.FirstOrDefault(x => x.SessionId == SessionId && x.Key == Key);
            }
        }

        /// <summary>
        /// Tries to find Cookie Value and Compare it with our matchwith value.
        /// </summary>
        /// <param name="SessionId">Session Id of owner</param>
        /// <param name="Key">Cookie known as</param>
        /// <param name="matchwith">Value to compare with</param>
        /// <returns>Wheter cookie exsist and value matches with input value</returns>
        public static bool ValueEquals(string SessionId, string Key, T matchwith)
        {
            T Value;

            if (!TryGetValue(SessionId, Key, out Value))
            {
                return false;
            }

            return Value.Equals(matchwith);
        }
    }

    public class Cookie<T> : IEquatable<string>
    {
        public Cookie(string sid, string key, T value)
        {
            SessionId = sid;
            Value = value;
            Key = key;
        }

        public string SessionId { get; private set; }
        public string Key { get; private set; }
        public T Value { get; private set; }

        public bool Equals(string other)
        {
            return string.Format("{0}-{1}", SessionId, Key).Equals(other);
        }

        public override bool Equals(object other)
        {
            if (ReferenceEquals(null, other)) return false;
            if (ReferenceEquals(this, other)) return true;
            if (!(other is Cookie<T>)) return false;
            return Equals((T)other);
        }

        public override string ToString()
        {
            return string.Format("{0}-{1}", SessionId, Key);
        }

        public override int GetHashCode()
        {
            int hash = 13;
            hash = (hash * 7) + SessionId.GetHashCode();
            hash = (hash * 7) + Key.GetHashCode();
            return hash;
        }
    }
}

Answer 1

Your implementation of the CookieRepository<T> is well written, easy to read and has good documentation, but it still can be beautyfied.

Style comments

• variables should be named using camelCase casing so the Cookies variable should be cookies. This should be done for method variables too. See the C# naming guidelines.
• method parameters should be named using camelCase casing too.
• IMHO braces {} should always be used and it won't hurt if you use them also where you put the execution statement on the same line like in the Equals() method.

The meat

The locking on an object which is involved in the processing is considered bad practice. You should always use a separate object to lock.

So replacing lock(Cookies), by using

 private static readonly object cookieLock = new object();  

, with lock(cookieLock) would be a good alternative to using the Cookies object.

---

public static bool Add(string SessionId, string Key, T Value)
{
    lock (Cookies)
    {
        if (Cookies.Any(x => x.SessionId == SessionId && x.Key == Key))
        {
            return false;
        }
        else
        {
            Cookies.Add(new Cookie<T>(SessionId, Key, Value));
        }

        return true;
    }
}  

here the else is redundant and should be removed, because if the condition is true the else won't be reached. By removing the else like so

public static bool Add(string SessionId, string Key, T Value)
{
    lock (Cookies)
    {
        if (Cookies.Any(x => x.SessionId == SessionId && x.Key == Key))
        {
            return false;
        }

        Cookies.Add(new Cookie<T>(SessionId, Key, Value));
        return true;
    }
}

will also save horizontal spacing, hence your code is more readable.

---

public static void RemoveAll(string SessionId)
{
    lock (Cookies)
    {
        var SessionCookies = Cookies.Where(x => x.SessionId == SessionId).ToList();

        foreach (var Cookie in SessionCookies)
        {
            Cookies.Remove(Cookie);
        }
    }
}  

By using the builtin RemoveWhere() method of the HashSet<T> performance will be increased and it will result in less code.

public static void RemoveAll(string SessionId)
{
    lock (Cookies)
    {
        Cookies.RemoveWhere(x => x.SessionId == SessionId);
    }
}  

---

/// <summary>
/// Adds or Replaces a cookie, used when ensurance that cookie by key does not already exist is of any importance.
/// </summary>
/// <param name="SessionId">Session Id of owner</param>
/// <param name="Key">Cookie known as</param>
/// <param name="Value">Value of the cookie</param>
public static void Set(string SessionId, string Key, T Value)  

Instead of stating in the summary Adds or Replaces a cookie you should name the method AddOrReplace() to make it clear what the method does.

Answer 2

I like the code and documentation in general, but I have a few minor nitpicks.

The reason the repository is static is that I wanted the ability to use the repository within Razor templates without having to use a reference object.

Having a non-static repository does not mean that you cannot use it in a static context. For example, the class could be non-static and could have a static Default property that could be use from inside Razor templates. That way, you could unit-test the cookie repository on it's own more easily.

public class CookieRepository<T>
{
    private static CookieRepository<T> instance;

    public static CookieRepository<T> Default
    {
        get
        {
            return instance ?? (instance = new CookieRepository<T>());
        }
    }

    // Rest of methods/variables, but non-static.
}

Usage: CookieRepository<bool>.Default.Add("myid", "Authenticated", true);

Using a HashSet<Cookie<T>> and querying it with LINQ might not be the best decision for performance. I may have chosen a IDictionary<string, IDictionary<string, Cookie<T>> (first key = sessionId, second key = cookie name), because dictionaries probably give you a better lookup performance than iterating through the complete Set with LINQ. If you use a ConcurrentDictionary<string, ConcurrentDictionary<string, Cookie<T>>, then you could use the thread-safety of this class and remove manual lock statements. For a lot of concurrent requests by different sessionIds, that could result in more locks but in a smaller scope (sessionId and key) each, so that response time for each cookie request/access can actually go down, because they do not have to wait for other sessions accessing the repository at the same time.

As the CookieRepository<T> is generic for it's cookie value types, you will automatically instantiate a different repository for each cookie value type that you use (think boolCookieRepository, stringCookieRepository, someComplexTypeRepository, …). If that's intentional, then that's okay, but IMHO, there is no good reason from an API usage point of view to have a single "repository" for each cookie value type.

The classes CookieRepository<T> and Cookie<T> are not named particularly precisely. They mostly mimick parts of IHttpSessionState with in-prcoess storage, and do not access browser cookies at all. Now, "cookie" is not a reserved word, but I know that if I had to look at this code in a webservice in production and saw a "cookie repository", then I might blindly assume that it worked with HTTP cookies. Maybe SessionValueCollection or SessionValueRepository and SessionValue would be better names?

 public override bool Equals(object other)
 {
     if (ReferenceEquals(null, other)) return false;
     if (ReferenceEquals(this, other)) return true;
     if (!(other is Cookie<T>)) return false;
     return Equals((T)other);
 }

This cannot work, because other cannot be cast to T. Either add an explicit cast operator to Cookie<T> or - and from the existing code, I think that is what you meant to do - return Equals(other.ToString()). Also, your Equals(string other) implementation only checks if sessionId and key match - one might say that it is unexpected for two Cookie<T> objects to be equal if they have the same sessionId and key, but different values.