4
\$\begingroup\$

I would like a review of the following application, esp with how I'm registering and selecting the controllers.

First I defined a generic Repository interface and implementation using EF 6.1.2:

public interface IRepository
{
    Task DeleteEntity(int id);
}
public interface IORepository<out TEntity> : IRepository
    where TEntity : class
{
    IEnumerable<TEntity> GetEntities();
    TEntity GetEntity(int id);
}
public interface IIRepository<in TEntity> : IRepository
    where TEntity : class
{
    Task SaveEntity(TEntity entity);
    Task AddEntity(TEntity entity);
}
public interface IRepository<TEntity> : IIRepository<TEntity>, IORepository<TEntity>
    where TEntity : class
{
    Task<IEnumerable<TEntity>> GetEntitiesAsync(CancellationToken token);
    Task<TEntity> GetEntityAsync(int id, CancellationToken token);
}
public class Repository<TEntity, TContext> : IRepository<TEntity>
    where TEntity : class
    where TContext : DbContext, new()
{
    protected TContext OpenContext()
    {
        return new TContext();
    }
    public virtual IEnumerable<TEntity> GetEntities()
    {
        using (var context = new TContext())
        {
            return context.Set<TEntity>().AsNoTracking().ToArray();
        }
    }

    public virtual async Task SaveEntity(TEntity entity)
    {
        using (var context = new TContext())
        {
            context.Set<TEntity>().Attach(entity);
            context.Entry(entity).State = EntityState.Modified;
            await context.SaveChangesAsync();
        }
    }

    public virtual async Task AddEntity(TEntity entity)
    {
        using (var context = new TContext())
        {
            context.Set<TEntity>().Add(entity);
            await context.SaveChangesAsync();
        }
    }

    public virtual async Task DeleteEntity(int id)
    {
        using (var context = new TContext())
        {
            var set = context.Set<TEntity>();
            set.Remove(set.Find(id));
            await context.SaveChangesAsync();
        }
    }

    public virtual TEntity GetEntity(int id)
    {
        using (var context = new TContext())
        {
            return context.Set<TEntity>().Find(id);
        }
    }

    public virtual async Task<IEnumerable<TEntity>> GetEntitiesAsync(CancellationToken token)
    {
        using (var context = new TContext())
        {
            return await context.Set<TEntity>().AsNoTracking().ToArrayAsync(token);
        }
    }

    public virtual async Task<TEntity> GetEntityAsync(int id, CancellationToken token)
    {
        using (var context = new TContext())
        {
            return await context.Set<TEntity>().FindAsync(token, id);
        }
    }
}
public class NorthwindRepository<TEntity> : Repository<TEntity, NorthwindContext> where TEntity : class { }

Then I defined a generic web api controller to handle a repository:

public interface IRepositoryController : IHttpController
{
    Task<HttpResponseMessage> GetAll();
    Task<HttpResponseMessage> Get(int id);
    Task<HttpResponseMessage> Delete(int id);
}
public interface IRepositoryController<in TEntity> : IRepositoryController
    where TEntity : class
{
    Task<HttpResponseMessage> Put(TEntity entity);
    Task<HttpResponseMessage> Post(TEntity entity);
}
public class RepositoryController<TEntity, TRepository> : ApiController, IRepositoryController<TEntity>
    where TEntity : class
    where TRepository : IRepository<TEntity>
{
    protected TRepository Repository { get; private set; }
    public RepositoryController(TRepository repository)
    {
        this.Repository = repository;
    }

    [HttpGet]
    public async virtual Task<HttpResponseMessage> GetAll()
    {
        try
        {
            CancellationTokenSource cancel = new CancellationTokenSource();
            var entities = await this.Repository.GetEntitiesAsync(cancel.Token);
            return Request.CreateResponse<IEnumerable<TEntity>>(entities);
        }
        catch (Exception ex)
        {
            return Request.CreateErrorResponse(HttpStatusCode.InternalServerError, ex.Message);
        }
    }
    [HttpGet]
    public virtual async Task<HttpResponseMessage> Get(int id)
    {
        try
        {
            CancellationTokenSource cancel = new CancellationTokenSource();
            var obj = await this.Repository.GetEntityAsync(id, cancel.Token);
            if (obj != null)
                return Request.CreateResponse<TEntity>(obj);
            return Request.CreateErrorResponse(HttpStatusCode.NotFound, "Item not found");
        }
        catch (Exception ex)
        {
            return Request.CreateErrorResponse(HttpStatusCode.InternalServerError, ex.Message);
        }
    }
    [HttpDelete]
    public virtual async Task<HttpResponseMessage> Delete(int id)
    {
        try
        {
            await this.Repository.DeleteEntity(id);
        }
        catch (Exception ex)
        {
            return Request.CreateErrorResponse(HttpStatusCode.InternalServerError, ex.Message);
        }
        return Request.CreateResponse(HttpStatusCode.OK);

    }
    [HttpPut]
    public virtual async Task<HttpResponseMessage> Put(TEntity entity)
    {
        try
        {
            await Repository.SaveEntity(entity);
        }
        catch (Exception ex)
        {
            return Request.CreateErrorResponse(HttpStatusCode.InternalServerError, ex.Message);
        }
        return Request.CreateResponse(HttpStatusCode.OK);
    }
    [HttpPost]
    public virtual async Task<HttpResponseMessage> Post(TEntity entity)
    {
        try
        {
            if (ModelState.IsValid)
            {
                await Repository.AddEntity(entity);
                return Request.CreateResponse<TEntity>(HttpStatusCode.Created, entity);
            }
            else
                return Request.CreateResponse(HttpStatusCode.InternalServerError, "Invalid model state");
        }
        catch (Exception ex)
        {
            return Request.CreateErrorResponse(HttpStatusCode.InternalServerError, ex.Message);
        }
    }
}

Then I defined a UnityDepdencyResolver and UnityControllerSelector. My biggest problem is with the controller selector where I instantiate the controller just to get it's type. I wish Unity had a way for me just to resolve type.

public class UnityResolver : IDependencyResolver
{
    private IUnityContainer Container { get; set; }
    public UnityResolver(IUnityContainer container)
    {
        this.Container = container;
    }

    public IDependencyScope BeginScope()
    {
        var child = this.Container.CreateChildContainer();
        return new UnityResolver(child);
    }

    public object GetService(Type serviceType)
    {
        try
        {
            return Container.Resolve(serviceType);
        }
        catch (ResolutionFailedException)
        {
            return null;
        }
    }

    public IEnumerable<object> GetServices(Type serviceType)
    {
        try
        {
            return this.Container.ResolveAll(serviceType);
        }
        catch (ResolutionFailedException)
        {
            return new object[] { };
        }
    }

    public void Dispose()
    {
        this.Container.Dispose();
    }
}

public class UnityControllerSelector : DefaultHttpControllerSelector, IDisposable
{
    private IUnityContainer Container { get; set; }
    private HttpConfiguration Configuration { get; set; }
    public UnityControllerSelector(HttpConfiguration configuration, IUnityContainer container) :base(configuration)
    {
        this.Container = container;
        this.Configuration = configuration;
    }

    public override HttpControllerDescriptor SelectController(System.Net.Http.HttpRequestMessage request)
    {
        var controllerName = base.GetControllerName(request);
        var controller = Container.Resolve<IHttpController>(controllerName);
        if(controller != null)
            return new HttpControllerDescriptor(this.Configuration, controllerName, controller.GetType());
        return base.SelectController(request);
    }

    public void Dispose()
    {
        this.Container.Dispose();
    }
}

I then defined an FilterProvider for authorization:

public class UnityFilterProvider : ActionDescriptorFilterProvider, IFilterProvider, IDisposable
{
    private IUnityContainer Container { get; set; }
    public UnityFilterProvider(IUnityContainer container)
    {
        this.Container = container;
    }
    IEnumerable<FilterInfo> IFilterProvider.GetFilters(HttpConfiguration configuration, HttpActionDescriptor actionDescriptor)
    {
        var filters = base.GetFilters(configuration, actionDescriptor);
        foreach (var filter in filters)
        {
            this.Container.BuildUp(filter.Instance.GetType(), filter.Instance);
        }
        var auth = this.Container.Resolve<RepositoryControllerAuthorizations>(actionDescriptor.ControllerDescriptor.ControllerName);
        if (auth != null)
        {
            List<FilterInfo> filterInfos = new List<FilterInfo>(filters);
            if (auth.ControllerFilter != null)
                filterInfos.Add(new FilterInfo(auth.ControllerFilter, FilterScope.Controller));
            if (auth.GetFilter != null && (actionDescriptor.ActionName == "Get" || actionDescriptor.ActionName == "GetAll"))
                filterInfos.Add(new FilterInfo(auth.GetFilter, FilterScope.Action));
            if (auth.DeleteFilter != null && actionDescriptor.ActionName == "Delete")
                filterInfos.Add(new FilterInfo(auth.DeleteFilter, FilterScope.Action));
            if (auth.PostFilter != null && actionDescriptor.ActionName == "Post")
                filterInfos.Add(new FilterInfo(auth.PostFilter, FilterScope.Action));
            if (auth.PutFilter != null && actionDescriptor.ActionName == "Put")
                filterInfos.Add(new FilterInfo(auth.PutFilter, FilterScope.Action));
            return filterInfos;
        }
        return filters;
    }

    public void Dispose()
    {
        this.Container.Dispose();
    }
}

public class RepositoryControllerAuthorizations
{
    public IFilter ControllerFilter { get; set; }
    public IFilter GetFilter { get; set; }
    public IFilter DeleteFilter { get; set; }
    public IFilter PostFilter { get; set; }
    public IFilter PutFilter { get; set; }
}

Then I made a RepositoryControllerRegistery class (I used the Inflectorlibrary found here to pluralize the entity's type name by default):

public static class RepositoryControllerRegistery<TEntity>
    where TEntity: class
{
    public static void Register<TRepositoryController>(IUnityContainer container, RouteCollection routes, string name = null, string path = "api", IFilter controllerFilter = null,
        IFilter getFilter = null, IFilter deleteFilter = null, IFilter putFilter = null, IFilter postFilter = null)
        where TRepositoryController : IRepositoryController<TEntity>
    {
        if(name == null)
            name = typeof(TEntity).Name.Pluralize();
        container.RegisterType<IHttpController, TRepositoryController>(name);
        RepositoryControllerAuthorizations auth = new RepositoryControllerAuthorizations()
        {
            ControllerFilter = controllerFilter,
            GetFilter = getFilter,
            DeleteFilter = deleteFilter,
            PutFilter = putFilter,
            PostFilter = postFilter
        };
        container.RegisterInstance<RepositoryControllerAuthorizations>(name, auth);
        routes.MapHttpRoute(string.Format("{0}GetAll", name), string.Format("{0}/{1}/GetAll", path, name), new { controller = name, action = "GetAll" });
        routes.MapHttpRoute(string.Format("{0}Get", name), string.Format("{0}/{1}/Get/{2}", path, name, "{id}"), new { controller = name, action = "Get" });
        routes.MapHttpRoute(string.Format("{0}Delete", name), string.Format("{0}/{1}/Delete/{2}", path, name, "{id}"), new { controller = name, action = "Delete" });
        routes.MapHttpRoute(string.Format("{0}Post", name), string.Format("{0}/{1}/Post", path, name), new { controller = name, action = "Post" });
        routes.MapHttpRoute(string.Format("{0}Put", name), string.Format("{0}/{1}/Put", path, name), new { controller = name, action = "Put" });
    }
    public static void Register(IUnityContainer container, RouteCollection routes, string name = null, string path = "api", IFilter controllerFilter = null,
        IFilter getFilter = null, IFilter deleteFilter = null, IFilter putFilter = null, IFilter postFilter = null)
    {
        Register<RepositoryController<TEntity>>(container, routes, name, path, controllerFilter, getFilter, deleteFilter, putFilter, postFilter);
    }
}

Then my Global.asax.cs looks like:

    protected void Application_Start()
    {
        UnityContainer container = new UnityContainer();
        container.RegisterType<IRepository<Product>, ProductRepository>();
        container.RegisterType<IRepository<Supplier>, NorthwindRepository<Supplier>>();
        container.RegisterType<IRepository<Employee>, NorthwindRepository<Employee>>();
        container.RegisterType<IRepository<Shipper>, NorthwindRepository<Shipper>>();
        container.RegisterType<IRepository<Order>, OrderRepository>();
        container.RegisterType<IRepository<Customer>, NorthwindRepository<Customer>>();
        RepositoryControllerRegistery<Product>.Register(container, RouteTable.Routes);
        RepositoryControllerRegistery<Supplier>.Register(container, RouteTable.Routes);
        RepositoryControllerRegistery<Employee>.Register(container, RouteTable.Routes);
        RepositoryControllerRegistery<Shipper>.Register(container, RouteTable.Routes);
        RepositoryControllerRegistery<Order>.Register(container, RouteTable.Routes);
        RepositoryControllerRegistery<Customer>.Register(container, RouteTable.Routes);
        AreaRegistration.RegisterAllAreas();

        GlobalConfiguration.Configuration.MapHttpAttributeRoutes();
        GlobalConfiguration.Configuration.DependencyResolver = new UnityResolver(container);
              GlobalConfiguration.Configuration.Services.Replace(typeof(IHttpControllerSelector), new UnityControllerSelector(GlobalConfiguration.Configuration, container));
       var providers = GlobalConfiguration.Configuration.Services.GetFilterProviders();
        var defaultProvider = providers.Single(i => i is ActionDescriptorFilterProvider);
             GlobalConfiguration.Configuration.Services.Remove(typeof(System.Web.Http.Filters.IFilterProvider), defaultProvider);
              GlobalConfiguration.Configuration.Services.Add(typeof(System.Web.Http.Filters.IFilterProvider), new UnityFilterProvider(container));
        GlobalConfiguration.Configure(WebApiConfig.Register);
        FilterConfig.RegisterGlobalFilters(GlobalFilters.Filters);
        RouteTable.Routes.MapMvcAttributeRoutes();

        RouteConfig.RegisterRoutes(RouteTable.Routes);
        BundleConfig.RegisterBundles(BundleTable.Bundles);
        GlobalConfiguration.Configuration.Formatters.XmlFormatter.SupportedMediaTypes.Clear();
    }

Then in TypeScript my Repository class looks like:

export interface IRepository {
    Delete(id: number): JQueryPromise<void>;
    ServiceLocation: string;
}
export interface IRepositoryGeneric<TEntity> extends IRepository {
    GetAll(): JQueryPromise<TEntity[]>;
    Get(id: number): JQueryPromise<TEntity>;
    Add(entity: TEntity): JQueryPromise<TEntity>;
    Update(entity: TEntity): JQueryPromise<void>;
}
export class Repository<TEntity> implements IRepositoryGeneric<TEntity>{
    constructor(public ServiceLocation: string) { }
    Delete(id: number): JQueryPromise<void> {
        return <JQueryPromise<void>>$.ajax({
            type: "DELETE",
            url: this.ServiceLocation + "Delete/" + id +"?time="+new Date().getTime()
        });
    }
    GetAll(): JQueryPromise<TEntity[]> {
        return <JQueryPromise<TEntity[]>>$.ajax({
            type: "GET",
            url: this.ServiceLocation + "GetAll" + "?time=" + new Date().getTime(),
        });
    }
    Get(id: number): JQueryPromise<TEntity> {
        return <JQueryPromise<TEntity>>$.ajax({
            type: "GET",
            url: this.ServiceLocation + "Get/" + id + "?time=" + new Date().getTime(),
        });
    }
    Add(entity: TEntity): JQueryPromise<TEntity> {
        return <JQueryPromise<TEntity>>$.ajax({
            type: "POST",
            url: this.ServiceLocation + "Post" + "?time=" + new Date().getTime(),
            data: JSON.stringify(entity),
            contentType: "application/json; charset=utf-8",
            dataType: 'json'
        });
    }
    Update(entity: TEntity): JQueryPromise<void> {
        return <JQueryPromise<void>>$.ajax({
            type: "PUT",
            url: this.ServiceLocation + "Put" + "?time=" + new Date().getTime(),
            data: JSON.stringify(entity),
            contentType: "application/json; charset=utf-8",
            dataType: 'json'
        });
    }
}

I also created an IoC for client side in TypeScript:

export interface IContainer {
    Register<TTo>(typeFrom: string, factory: (arguments?: any[]) => TTo, name?: string, injectionMembers?: IConstructorParameterFactory[]);
    Resolve<T>(type: string, name?: string, resolveOverrides?: IConstructorParameterFactory[]): T;
}
export interface IConstructorParameterFactory {
    Construct(): any;
    Name: string;
}
export class ConstructorParameterFactory implements IConstructorParameterFactory {
    constructor(public Name: string, private factory: () => any) { }
    Construct(): any {
        return this.factory();
    }
}
export class DefaultConstructorFactory implements IConstructorParameterFactory {
    constructor(public Name: string) { }
    Construct(): any {
        return null;
    }
}
export class TypedConstructorFactory implements IConstructorParameterFactory {
    constructor(public Name: string, private type: string, private container: IContainer, private typeParameterName?: string, private resolveOverides?: IConstructorParameterFactory[]) {
    }
    Construct(): any {
        return this.container.Resolve(this.type, this.typeParameterName, this.resolveOverides);
    }
}
class Dependency {
    private injectionParameters: Lind.Collections.Dictionary<IConstructorParameterFactory>;
    constructor(private factory: (parameters?: any[]) => any, parameters?: IConstructorParameterFactory[]) {
        this.injectionParameters = new Lind.Collections.Dictionary<IConstructorParameterFactory>();
        if (parameters != null) {
            for (var i: number = 0; i < parameters.length; i++) {
                this.injectionParameters.add(parameters[i].Name, parameters[i]);
            }
        }
    }
    Resolve<T>(parms?: IConstructorParameterFactory[]) : T {
        var p: Lind.Collections.Dictionary<IConstructorParameterFactory> = new Lind.Collections.Dictionary<IConstructorParameterFactory>();
        var ip = this.injectionParameters.values();
        for (var i: number = 0; i < ip.length; i++) {
            p.add(ip[i].Name, ip[i]);
        }
        if (parms != null) {
            for (var i: number = 0; i < parms.length; i++) {
                p.setValue(parms[i].Name, parms[i]);
            }
        }
        var arguments: any[] = [];
        var pv = p.values();
        for (var i: number = 0; i < pv.length; i++) {
            arguments.push(pv[i].Construct());
        }
        return <T>this.factory(arguments);
    }
}
export class Container implements IContainer {
    private namedDepdencies: Lind.Collections.Dictionary<Lind.Collections.Dictionary<Dependency>> = new Lind.Collections.Dictionary<Lind.Collections.Dictionary<Dependency>>();
    private dependencies: Lind.Collections.Dictionary<Dependency> = new Lind.Collections.Dictionary<Dependency>();
    Register<TTo>(typeFrom : string, factory: (arguments?: any[]) => TTo, name?: string, injectionMembers?: IConstructorParameterFactory[]) {
        var dependency = new Dependency(factory, injectionMembers);
        if (name != null) {
            if (!this.namedDepdencies.containsKey(typeFrom))
                this.namedDepdencies.add(typeFrom, new Lind.Collections.Dictionary<Dependency>());
            var typeDictionary = this.namedDepdencies.getValue(typeFrom);
            if (!typeDictionary.containsKey(name))
                typeDictionary.add(name, dependency);
            else
                typeDictionary.setValue(name, dependency);
        }
        else {
        if (!this.dependencies.containsKey(typeFrom))
            this.dependencies.add(typeFrom, dependency);
        else
            this.dependencies.setValue(typeFrom, dependency);
        }
    }
    Resolve<T>(type:string, name?: string, resolveOverrides?: IConstructorParameterFactory[]): T {
        if (name != null) {
            return this.namedDepdencies.getValue(type).getValue(name).Resolve<T>(resolveOverrides);
        }
        else
            return this.dependencies.getValue(type).Resolve<T>(resolveOverrides);
    }
}

Which the main view uses as followed:

export class MainWindowView {
    constructor(public ServiceLocationBase: string) {
        var container = new Lind.IoC.Container();
        container.Register("ProductRepository", (arguments => new Northwind.Repository.Repository<Northwind.IProduct>(<string>arguments[0])), null,
            [new Lind.IoC.ConstructorParameterFactory("ServiceLocation", () => this.ServiceLocationBase + "Products/")]);
        container.Register("SupplierRepository", (arguments => new Northwind.Repository.Repository<Northwind.ISupplier>(<string>arguments[0])), null,
            [new Lind.IoC.ConstructorParameterFactory("ServiceLocation", () => this.ServiceLocationBase + "Suppliers/")]);

        container.Register(typeof ViewModels.Navigation.NavigationItem,
            (arguments) => new ViewModels.Repository.ProductsNavigationItem(<ViewModels.Navigation.INavigationData>arguments[0],
                <Northwind.Repository.IRepositoryGeneric<Northwind.IProduct>>arguments[1]),
            "Products",
            [
                new Lind.IoC.DefaultConstructorFactory("data"),
                new Lind.IoC.TypedConstructorFactory("repository", "ProductRepository", container)
            ]);
        container.Register(typeof ViewModels.Navigation.NavigationItem,
            (arguments) => new ViewModels.Repository.SuppliersNavigationItem(<ViewModels.Navigation.INavigationData>arguments[0],
                <Northwind.Repository.IRepositoryGeneric<Northwind.ISupplier>>arguments[1]),
            "Suppliers",
            [
                new Lind.IoC.DefaultConstructorFactory("data"),
                new Lind.IoC.TypedConstructorFactory("repository", "SupplierRepository", container)
            ]);
        ViewModels.Navigation.NavigationItemFactory.Initalize(data => container.Resolve<ViewModels.Navigation.NavigationItem>(typeof ViewModels.Navigation.NavigationItem, data.Name,
            [new Lind.IoC.ConstructorParameterFactory("data", () => data)]));
        this.ViewModel = new ViewModels.MainWindow.MainWindowViewModel([this.createNavigationData("Products"), this.createNavigationData("Suppliers")]);
        ko.applyBindings(this.ViewModel);
    }
    private createNavigationData(name: string, displayName?: string) : ViewModels.Navigation.INavigationData {
        if (displayName == null)
            displayName = name;
        return new ViewModels.Navigation.NavigationData(name, displayName, false); 
    }
    public ViewModel: ViewModels.MainWindow.MainWindowViewModel;
}

You can find the whole project on github: https://github.com/jasonlind0/Lind.Web.Mvvm (be forewarned it's POC code and not very well organized into a framework)

\$\endgroup\$
1
\$\begingroup\$

Your UnityResolver class has a Container field that implements IDisposable. Running Visual Studio's code analysis tool will tell you

Types that own disposable fields should be disposable.

For more information, see CA1001.

You do have a Dispose method in the class, but it's not implementing IDisposable. You should include it in your interface list. Of course, doing this will raise another Code Analysis warning that you've not implemented IDisposable correctly. For more information on that, please see CA1063.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ IDepdencyResolver extends IDisposable so that's not a problem. But I noticed there were a few other classes where I'm not properly disposing of the container. It's been updated. \$\endgroup\$ – JasonLind Mar 3 '15 at 16:49
  • \$\begingroup\$ I'm glad I could help (even if I was apparently wrong), but please refrain from updating the code in your question after answers have been posted. \$\endgroup\$ – RubberDuck Mar 3 '15 at 16:52
  • \$\begingroup\$ I would also question the practice of extending interfaces over implementing multiple interfaces as, obviously, it can cause confusion. \$\endgroup\$ – RubberDuck Mar 3 '15 at 16:54
  • \$\begingroup\$ No interface extension is critical in OO to polymorph and also enforce generic constraints \$\endgroup\$ – JasonLind Mar 3 '15 at 17:22
0
\$\begingroup\$

Very smart idea. Although I prefer to use your generic API controller directly without having to register concrete entity types. Professionals said that direct usage of generic repository is kind of anti pattern scenario, but I don't believe in. Your smart idea in conjunction with OData action filter translated into SQL queries could be an other point of view. Instead reading values from database and populating objects and after that reading again values from those objects and producing JSON using some JSON Serializer, we can directly create a JSON string from DataReader and avoid unnecessary creation of objects.

|improve this answer|||||
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.