Get Unique Code using Singleton - ASP.NET Core

Question

I am using ASP.NET Core 1.1 with Entity Framework Core 1.1.

I try with below code to get unique "Item Code" whenever user creates item into the system. This case is about to handle concurrency / multiple request for item creation. I have done similar to this using "Singleton" class in ASP.NET, but in case of ASP.NET CORE, I have got duplicate values(only at few instance) for "Item Code". Hence just want to make sure that the code here is correct with respect to getting different "Item Code" for each request.

----------- Injecting Dependency ---------------

namespace Microsoft.Extensions.DependencyInjection
{
   public static class StartupExtensions
   {
     public static IServiceCollection AddCloudscribeCore(this IServiceCollection services, IConfigurationRoot configuration)
    {
        // Add application services.            

        services.AddSingleton<ISingletonService, SingletonService>(); // this will create singleton instance of class.
        return services;
    }
  }
}

----- Singleton Service -------------------

public class SingletonService : ISingletonService
{
    private readonly AppDbContext _context;

    public SingletonService(AppDbContext context)
    {
        _context = context;
    }


    public CommonResult GetUniqueCode(Guid tenantId)
    {
        CommonResult result = new CommonResult();
        string newCode = string.Empty;

        var query = (from tenant in _context.Tenants
                         join item in _context.InventoryItems on tenant.TenantId equals item.TenantId into tenantItemJoin
                         from subItem in tenantItemJoin.DefaultIfEmpty()
                         where tenant.TenantId == tenantId || subItem.ItemCode.Contains("I-")
                         orderby subItem.CreatedUtc descending
                         select new TenantItem
                         {
                             TenantCode = tenant.TenantCode,
                             ItemCode = (subItem != null ? (subItem.ItemCode == string.Empty ? string.Empty:subItem.ItemCode) : string.Empty)
                         }).FirstOrDefault();
        if (query  != null)
        {


            string tenantCode = query.TenantCode.Substring(2);
            if (!string.IsNullOrEmpty(query.ItemCode))
            {
                int code = Convert.ToInt32(query.ItemCode.Substring(2)) + 1;
                if (code < 10)
                    newCode = "I-00000" + code;
                else if (code >= 10 && code < 100)
                    newCode = "I-0000" + code;
                else if (code >= 100 && code < 1000)
                    newCode = "I-000" + code;
                else if (code >= 1000 && code < 10000)
                    newCode = "I-00" + code;
                else if (code >= 10000 && code < 100000)
                    newCode = "I-0" + code;
                else if (code >= 100000 && code < 1000000)
                    newCode = "I-" + code;
                else
                    newCode = "I-" + code;

            }
            else
            {
                newCode = "I-000001";
            }
            TenantItem tenantItem = new TenantItem();
            tenantItem.TenantCode = tenantCode;
            tenantItem.ItemCode = newCode;
            result.Succeeded = true;
            result.Object = tenantItem;

        }

        return result;
    }
}

------------- API Controller -------------

[HttpGet]
public CommonResult GetItemCode()
{
    CommonResult result = new CommonResult();
    result = _singletonService.GetItemCode(tenantId);
    TenantItem tenantItem = (TenantItem)result.Object;
}

If I see the output/values in database, I see couple of entries(where the data-entry time difference is in milliseconds) have duplicate ID. See below extracts,

ItemCode CreatedUTC 
I-000045 2017-10-31 11:06:10.6419557
I-000045 2017-10-31 11:06:10.5482045
I-000044 2017-10-31 11:03:58.0772064
I-000043 2017-10-31 11:03:57.7803288
I-000042 2017-10-31 11:03:04.1054090
I-000042 2017-10-31 11:03:03.9632107
I-000032 2017-10-25 17:39:04.7054946
I-000031 2017-10-25 17:34:34.7300091 
I-000030 2017-10-25 17:24:15.8891966 
I-000029 2017-10-25 17:19:36.2187677 
I-000028 2017-10-25 17:06:19.8946515 
I-000027 2017-10-25 17:03:45.8059024 
I-000026 2017-10-25 11:48:59.2262869
I-000025 2017-10-25 11:47:15.5935031 
I-000024 2017-10-25 11:45:31.8251470 
I-000023 2017-10-25 11:43:55.6671755 
I-000022 2017-10-25 06:33:27.2546438 
I-000019 2017-10-24 11:27:05.2224204
I-000016 2017-10-24 10:21:09.5983741
I-000015 2017-10-24 10:18:54.7954042 
I-000011 2017-10-24 09:19:04.9033847
I-000010 2017-10-24 09:17:29.0905153
I-000008 2017-10-23 11:48:12.0188814
I-000007 2017-10-23 08:50:17.0785334 
I-000006 2017-10-23 08:46:36.6120703 
I-000005 2017-10-23 08:01:15.3829637

Please suggest if I am missing anything here.

score 1 · Accepted Answer · answered Nov 07 '17 at 14:37

This is why it's better to let the database handle identity-style columns. Regardless, this doesn't have anything to do with singletons and such; it's simple database concurrency. Singleton or not, you're running a query on a database table to fetch the latest "id", incrementing it, and saving a new record. However, unless you issue a lock on the table (which is a bad idea, by the way), other requests invoking this same operation could be being carried out. Long and short, your code is not thread-safe.

The way to make it thread safe is to either 1) create locks or 2) rely on a unique constraint and employ a catch-and-retry policy. Since #1 is almost always a bad idea, especially in a database scenario, #2 should be your preferred path.

Simply, if this value should be unique, then there should be a unique constraint on the column in the database. Since you're able to actually create duplicates, that's obviously not the case. However, if you correct that, then attempt to save a dupe will result in a database exception, which will bubble up and can be caught by your application. You can then catch this exception and retry the operation, fetching the latest "id", again, incrementing that, again, and attempting to save, again. Rinse and repeat until it goes through without issue.

This does mean that the operation could take a little longer to complete, as it may have to do multiple roundtrips to the database before it can nail down an available "id", but it's the only safe way to accomplish this, if you refuse to use an identity column.

Thanks, I'll change the implementation as suggested. – Arayn Nov 08 '17 at 13:12 — Arayn, Nov 08 '17 at 13:12

AperioOculus · Answer 2 · 2017-11-07T15:52:43.253

1. Your database context should not be embedded in a service using the singleton lifetime. Unless you specified otherwise when registering your AppDbContext, it is set to the 'scoped' lifetime. I recommend you also use a scoped lifespan for the service that contains your database context.

2. I would allow the database to set the unique Id for your item code. It has builtin mechanisms to support concurrency and ensure uniqueness. In my opinion it makes sense to leverage those. You can use an auto-incrementing Id on your InventoryItem table and either mark it as the primary key or add a unique constraint on the column.

References:

Get Unique Code using Singleton - ASP.NET Core

2 Answers2