Controller refactoring question

Question

User-1104215994 posted

Hi guys,

I have a rest asp.net web api. I am querying database to check if the data is unique. If the data has used before, I will return bad request. Since I will use this same query more than one method, can I make it a function? I have a Utilities class under the Utilities folder in my project. I would like to make a static async function for this query. Should I send context as a parameter? What would be the proper way?

Here is relevant portion of my code:

[System.Web.Http.RoutePrefix("api/v2/pin")]
    public class InitiatesController : ApiController
    {
        private readonly EPINMiddleWareAPIContext context;

        public InitiatesController(EPINMiddleWareAPIContext context)
        {
            this.context = context;
        }


        // POST: api/Game
        //[RequireHttps] For Prod Only
        [System.Web.Http.HttpPost, System.Web.Http.Route("initiation")]
        public async Task<IHttpActionResult> PostInitiate(InitiateRequest initiate)
        {
            #region Is referenceId Unique?
            //Is referenceId is unique in our database
            var uniqueID =
                await (context.InitiateRequests.Where(p => p.referenceId == initiate.referenceId))
                    .ToListAsync();
            if (uniqueID.Any())
            {
                ModelState.AddModelError("referenceId", "This referenceId has used!");
            }
            
            #endregion
            if (!ModelState.IsValid)
            {
                return BadRequest(ModelState);
            }

Answer 1

User475983607 posted

I have a Utilities class under the Utilities folder in my project. I would like to make a static async function for this query. Should I send context as a parameter?

It's perfectly fine to pass a DbContext to a static method.  Are you receiving errors? 

If the validation check exists in a single controller then I would add the method to the controller.

Answer 2

User-1104215994 posted

Dbcontext still be <g class="gr_ gr_15 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar only-ins replaceWithoutSep" id="15" data-gr-id="15">disposed</g> if I pass it to the method?

Answer 3

User-1104215994 posted

Declared IsUnique method in the <g class="gr_ gr_36 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="36" data-gr-id="36">Utilties</g> class and added the method to the controller action. I wonder if I can make it <g class="gr_ gr_103 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="103" data-gr-id="103">aync</g>?

public InitiatesController(EPINMiddleWareAPIContext context)
        {
            this.context = context;
        }

        // POST: api/Game
        //[RequireHttps] For Prod Only
        [System.Web.Http.HttpPost, System.Web.Http.Route("initiation")]
        public async Task<IHttpActionResult> PostInitiate(InitiateRequest initiate)
        {
            #region Is referenceId Unique?
            
            if (Utilities.IsUnique(context, initiate)>0)
            {
                ModelState.AddModelError("referenceId", "This referenceId has used!");
            }
            
            #endregion
            if (!ModelState.IsValid)
            {
                return BadRequest(ModelState);
            }

public static int IsUnique(EPINMiddleWareAPIContext context, InitiateRequest initiate)
        {
            //Is referenceId is unique in our database
            var uniqueID =
                (context.InitiateRequests.Where(p => p.referenceId == initiate.referenceId))
                    .ToList();
            return uniqueID.Count();
        }

Answer 4

User475983607 posted

Dbcontext still be disposed if I pass it to the method?

You're passing a DbContext reference not the DbContext.  The class that created the DbContext is responsible for disposing the DbContext.

Answer 5

User475983607 posted

I wonder if I can make it aync?

Did you try to create an async method and are now receiving an error?  If so, what is the error and can you share the source code?

Answer 6

User-1104215994 posted

this way it is working. Seems OK?

 public static async Task<int> IsUnique(EPINMiddleWareAPIContext context, InitiateRequest initiate)
        {
            //Is referenceId is unique in our database
            var uniqueID =
                await (context.InitiateRequests.Where(p => p.referenceId == initiate.referenceId))
                    .ToListAsync();
           
            return uniqueID.Count();
        }

Here is the controller:

if (await Utilities.IsUnique(context, initiate)>0)
            {
                ModelState.AddModelError("referenceId", "This referenceId has used!");
            }
            
            #endregion
            if (!ModelState.IsValid)
            {
                return BadRequest(ModelState);
            }

Answer 7

User765422875 posted

Putting your re-usable data access code into a separate class/method is the right thing to do.

I would change your code so it is more readable.

var count = await Utilities.IsUnique(context, initiate);

if (count > 0)
{
  // your code
}

Also, make sure you are properly disposing the Context.

Answer 8

User-1104215994 posted

But as <g class="gr_ gr_11 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="11" data-gr-id="11">mgebhard</g> said "You're passing a DbContext reference <g class="gr_ gr_20 gr-alert gr_gramm gr_inline_cards gr_run_anim Punctuation only-ins replaceWithoutSep" id="20" data-gr-id="20">not</g> the DbContext. The class that created the DbContext is responsible for <g class="gr_ gr_19 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar only-ins replaceWithoutSep" id="19" data-gr-id="19">disposing</g> the DbContext." DbContext is in the controller and I am passing it to the method. Controller <g class="gr_ gr_188 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar only-ins replaceWithoutSep" id="188" data-gr-id="188">disposing</g> it right?

Answer 9

User765422875 posted

Yes and No. Microsoft claims that the default behavior of DbContext is that the underlying connection is automatically opened any time it is needed and closed when it is no longer needed. With EF, in particular, heavy re-usage of contexts, or multiple uses without Disposing can cause major performance problems as you scale.

Are you using an IoC container to inject it? How is the DbContext setup? In my opinion, the safest way is to dispose of the context is in a using statement. 

Answer 10

User-474980206 posted

But as mgebhard said "You're passing a DbContext reference not the DbContext. The class that created the DbContext is responsible for disposing the DbContext." DbContext is in the controller and I am passing it to the method. Controller disposing it right?

yes, if coded correctly.

if the controller construction creates the dbcontext, then the controller needs to supply a dispose method that call dispose on the dbcontext. if the dbcontext is passed to the controller via DI, then the DI subsystem is responsible for the dispose.

 

Answer 11

User753101303 posted

Hi,

We would need first to know who is passing the context to your controller. You are using dependency injection ? It seems you think that passing an object to a method could change the way it is disposed. I'm not sure what make you think that ?

Ah 

DbContext is in the controller

Controller disposing it

It seems you are perhaps thinking that the method in which a parameter or variable is declared is responsible for disposing an object ??? No.

For now it seems you are using Dependency Injection and you likekly told the DbContext lifetime is scoped to the http query and so the DI container should dispose the DbContext at the end of the Http query. As you have more and more open source code it might be even visible in the code for your DI container (or at least you could track f Dispose is called and from where).

So once again who is providing the DbContext instance to your controller ?

Answer 12

User-1104215994 posted

So once again who is providing the DbContext instance to your <g class="gr_ gr_8 gr-alert gr_gramm gr_inline_cards gr_run_anim Style multiReplace" id="8" data-gr-id="8">controller ?</g>

Here is my controller:

public class InitiatesController : ApiController
    {
        private readonly EPINMiddleWareAPIContext context;

        public InitiatesController(EPINMiddleWareAPIContext context)
        {
            this.context = context;
        }

And here is the API context:

public class EPINMiddleWareAPIContext : DbContext
    {
        

        public EPINMiddleWareAPIContext() : base("name=EPINMiddleWareAPIContext")
        {
        }

        public DbSet<InitiateRequest> InitiateRequests { get; set; }
        public DbSet<InitiateResponse> InitiateResponses { get; set; }
        public DbSet<Company> Companies { get; set; }
        public DbSet<ConfirmRequest> ConfirmRequests { get; set; }
        public DbSet<ConfirmResponse> ConfirmResponses { get; set; }
        public DbSet<GameBank> GameBanks { get; set; }
        public DbSet<GameCouponBank> GameCouponBanks { get; set; }

Answer 13

User475983607 posted

You did not answer the question!  Rather you supplied incomplete code that tell us if you are using DI or not.  Is there any reason why you don't simply answer the question?

Answer 14

User-1104215994 posted

yes I m using DI, isn't it clear?

[System.Web.Http.RoutePrefix("api/v2/pin")]
    public class InitiatesController : ApiController
    {
        private readonly EPINMiddleWareAPIContext context;

        public InitiatesController(EPINMiddleWareAPIContext context)
        {
            this.context = context;
        }



        // POST: api/Game
        //[RequireHttps] For Prod Only
        [System.Web.Http.HttpPost, System.Web.Http.Route("initiation")]
        public async Task<IHttpActionResult> PostInitiate(InitiateRequest initiate)
        {
            #region Is referenceId Unique?
           
            if (await Utilities.IsUnique(context, initiate) > 0)
            {
                ModelState.AddModelError("referenceId", "This referenceId has used!");
            }

Answer 15

User765422875 posted

You are injecting the context. But people are asking for the code that sets it up. Are you using ASP.NET Core and the built-in IoC container?

Answer 16

User475983607 posted

yes I m using DI, isn't it clear?

Well no.   A constructor with an input parameter does not indicate DI.  That's why I asked.  Please do not answer forum posts with cryptic response.  Just answer the question.

Answer 17

User-1104215994 posted

Apologize for misdirection, sorry my bad. There is no DI.

Answer 18

User765422875 posted

Then it seems you are not properly disposing of the DbContext. Like I said in a previous answer - "Microsoft claims that the default behavior of DbContext is that the underlying connection is automatically opened any time it is needed and closed when it is no longer needed. With EF, in particular, heavy re-usage of contexts, or multiple uses without Disposing can cause major performance problems as you scale."

You could dispose of the context in the controller, but in my opinion, it's better to create and dispose of the db context just where you have to. These are your options unless you are using an IoC. Hope this helps.

Controller Option (example)

public class MyController : Controller
{
    private MyContext db = new MyContext();

    protected override void Dispose(bool disposing)
    {
        if (disposing)
        {
            db.Dispose();
        }
        base.Dispose(disposing);
    }
}

Using Option (example)

using (var db = new MyContext())
{...}

Answer 19

User-1104215994 posted

I used <g class="gr_ gr_7 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="7" data-gr-id="7">ninject</g>, here is my Ninject.Web.Common file. I guess now it is safe to send Dbcontext from <g class="gr_ gr_169 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar only-ins replaceWithoutSep" id="169" data-gr-id="169">controller</g> to other methods inside of <g class="gr_ gr_217 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar multiReplace" id="217" data-gr-id="217">Utilities</g> class since the disposal is managed by the <g class="gr_ gr_305 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="305" data-gr-id="305">ninject</g> right?

[assembly: WebActivatorEx.PreApplicationStartMethod(typeof(EPINMiddleWareAPI.App_Start.NinjectWebCommon), "Start")]
[assembly: WebActivatorEx.ApplicationShutdownMethodAttribute(typeof(EPINMiddleWareAPI.App_Start.NinjectWebCommon), "Stop")]

namespace EPINMiddleWareAPI.App_Start
{
    using System;
    using System.Web;

    using Microsoft.Web.Infrastructure.DynamicModuleHelper;

    using Ninject;
    using Ninject.Web.Common;
    using Models;
    using Ninject.Web.Common.WebHost;

    public static class NinjectWebCommon 
    {
        private static readonly Bootstrapper bootstrapper = new Bootstrapper();

        /// <summary>
        /// Starts the application
        /// </summary>
        public static void Start() 
        {
            DynamicModuleUtility.RegisterModule(typeof(OnePerRequestHttpModule));
            DynamicModuleUtility.RegisterModule(typeof(NinjectHttpModule));
            bootstrapper.Initialize(CreateKernel);
        }
        
        /// <summary>
        /// Stops the application.
        /// </summary>
        public static void Stop()
        {
            bootstrapper.ShutDown();
        }
        
        /// <summary>
        /// Creates the kernel that will manage your application.
        /// </summary>
        /// <returns>The created kernel.</returns>
        private static IKernel CreateKernel()
        {
            var kernel = new StandardKernel();
            try
            {
                kernel.Bind<Func<IKernel>>().ToMethod(ctx => () => new Bootstrapper().Kernel);
                kernel.Bind<IHttpModule>().To<HttpApplicationInitializationHttpModule>();
                RegisterServices(kernel);
                return kernel;
            }
            catch
            {
                kernel.Dispose();
                throw;
            }
        }

        /// <summary>
        /// Load your modules or register your services here!
        /// </summary>
        /// <param name="kernel">The kernel.</param>
        private static void RegisterServices(IKernel kernel)
        {
            kernel.Bind<EPINMiddleWareAPIContext>().To<EPINMiddleWareAPIContext>();
        }        
    }
}

Answer 20

User36583972 posted

Hi cenk1536,

I used ninject, here is my Ninject.Web.Common file. I guess now it is safe to send Dbcontext from controller to other methods inside of Utilities class since the disposal is managed by the ninject right?

Ninject.Web.Common provides the base infrastructure for all web type extension.

You can visit the Ninject.Web.Common issue for getting suitable help.

Best Regards

Yong Lu

Answer 21

User-1104215994 posted

In <g class="gr_ gr_3 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="3" data-gr-id="3">ninject</g> should I bind context to <g class="gr_ gr_4 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar only-ins doubleReplace replaceWithoutSep" id="4" data-gr-id="4">controller</g>?

private static void RegisterServices(IKernel kernel)
        {
            kernel.Bind<EPINMiddleWareAPIContext>().To<EPINMiddleWareAPIContext>();
        }       

Or something like this:

kernel.Bind<EPINMiddleWareAPIContext>().ToSelf().InRequestScope();