Ok, I have a serious issue now. I have been using a service / repo / unitofwork design pattern for a couple of years now and I have never had any issues. But for some reason the latest project it is causing me no end of problems. I am not sure whether it is just me not seeing clearly or whether the pattern just doesn't fit this project. So, here goes.
I have a POCO class marked like this:
public class Kit
{
public int Id { get; set; }
public string Name { get; set; }
public int TotalPrice { get; set; }
public Design Design { get; set; }
public Team Team { get; set; }
public IList<KitColour> Colours { get; set; }
public IList<Template> Templates { get; set; }
}
The problem I have is with the navigation properties. Originally it was with the Colours lookup table. As you can see from the model above I use a List of KitColour which looks like this:
public class KitColour
{
public int KitId { get; set; }
public string ColourId { get; set; }
}
In actual fact I wanted to use the normal POCO class Colour:
public class Colour
{
public string Id { get; set; }
public string Name { get; set; }
}
but I found because there was a many to many relationship with Kit and Colour I had to create a lookup table (KitColours) which just has two foreign keys which I had mapped like this:
// Map the KitColours table
modelBuilder.Entity<Kit>()
.HasMany(m => m.Colours)
.WithMany()
.Map(m =>
{
m.MapLeftKey("KitId");
m.MapRightKey("ColourId");
m.ToTable("KitColours");
});
This was fine for retrieving data, but for inserting data this caused me some issues. Before I go into the actual service for the Kit, here are my Repository, Service and UnitOfWork classes:
public class Repository<T> : IDisposable, IRepository<T> where T : class
{
private readonly DbContext context;
private readonly DbSet<T> dbEntitySet;
public Repository(DbContext context)
{
if (context == null)
throw new ArgumentNullException("context");
this.context = context;
this.dbEntitySet = context.Set<T>();
}
public IQueryable<T> GetAll(params string[] includes)
{
IQueryable<T> query = this.dbEntitySet;
foreach (var include in includes)
query = query.Include(include);
return query;
}
public void Create(T model)
{
this.dbEntitySet.Add(model);
}
public void Update(T model)
{
this.context.Entry<T>(model).State = EntityState.Modified;
}
public void Remove(T model)
{
this.context.Entry<T>(model).State = EntityState.Deleted;
}
public void Dispose()
{
this.context.Dispose();
}
}
public class Service<T> where T : class
{
private readonly IUnitOfWork unitOfWork;
private readonly IRepository<T> repository;
protected IUnitOfWork UnitOfWork
{
get { return this.unitOfWork; }
}
protected IRepository<T> Repository
{
get { return this.repository; }
}
public Service(IUnitOfWork unitOfWork)
{
if (unitOfWork == null)
throw new ArgumentNullException("unitOfWork");
this.unitOfWork = unitOfWork;
this.repository = unitOfWork.GetRepository<T>();
}
}
public class UnitOfWork<TContext> : IUnitOfWork where TContext : DbContext, new()
{
private readonly DbContext context;
private Dictionary<Type, object> repositories;
public DbContext Context { get { return this.context; } }
public UnitOfWork()
{
this.context = new TContext();
repositories = new Dictionary<Type, object>();
}
public IRepository<TEntity> GetRepository<TEntity>() where TEntity : class
{
if (repositories.Keys.Contains(typeof(TEntity)))
return repositories[typeof(TEntity)] as IRepository<TEntity>;
var repository = new Repository<TEntity>(context);
repositories.Add(typeof(TEntity), repository);
return repository;
}
public async Task SaveChangesAsync()
{
try
{
await this.context.SaveChangesAsync();
}
catch (DbUpdateConcurrencyException ex)
{
ex.Entries.First().Reload();
}
}
public void Dispose()
{
this.context.Dispose();
}
}
So my KitService class inherits from Service and looks like this:
/// <summary>
/// Kit service handles all kit related functions
/// </summary>
public class KitService : Service<Kit>
{
private readonly KitColourService kitColourService;
/// <summary>
/// Default constructor
/// </summary>
/// <param name="unitOfWork"></param>
public KitService(IUnitOfWork unitOfWork, KitColourService kitColourService)
: base(unitOfWork)
{
this.kitColourService = kitColourService;
}
/// <summary>
/// Get all kits asynchronously
/// </summary>
/// <param name="includes">Eager loading includes</param>
/// <returns>A list of colours</returns>
public async Task<IList<Kit>> GetAllAsync(params string[] includes)
{
return await this.Repository.GetAll(includes).ToListAsync();
}
/// <summary>
/// Get a kit by id
/// </summary>
/// <param name="id">The id of the colour</param>
/// <param name="includes">Eager loading includes</param>
/// <returns>A colour</returns>
public async Task<Kit> GetAsync(int id, params string[] includes)
{
var models = await this.GetAllAsync(includes);
return models.Where(model => model.Id == id).SingleOrDefault();
}
/// <summary>
/// Create a kit
/// </summary>
/// <param name="model">The kit model</param>
public async Task CreateAsync(Kit model)
{
// Create a kit
this.Repository.Create(model);
// Save the database changes
await this.UnitOfWork.SaveChangesAsync();
}
/// <summary>
/// Update a kit
/// </summary>
/// <param name="model">The kit model</param>
/// <param name="colours">The colours to be added to the kit</param>
public async Task UpdateAsync(Kit model, IList<Colour> colours)
{
// Update a kit
this.Repository.Update(model);
// For each colour that has to be removed, remove from our kit colours
if (model.Colours != null)
foreach (var colour in model.Colours)
if (!colours.Any(c => c.Id == colour.ColourId))
this.kitColourService.Remove(new KitColour { ColourId = colour.ColourId, KitId = model.Id });
// For each colour that has to be added, add to our kit colours
if (colours != null)
foreach (var colour in colours)
if (!model.Colours.Any(c => c.ColourId == colour.Id))
this.kitColourService.Create(new KitColour { ColourId = colour.Id, KitId = model.Id });
// Save the database changes
await this.UnitOfWork.SaveChangesAsync();
}
/// <summary>
/// Delete a kit
/// </summary>
/// <param name="model">The kit model</param>
public void Remove(Kit model)
{
// Remove a kit
this.Repository.Remove(model);
}
}
Now, you will see here that for the Update method I have another parameter called colours which I pass. The KitService has a readonly property for another service called KitColourService which only exposes Create and Remove. I had to do it this way because if I just did something like:
public class Kit
{
public int Id { get; set; }
public string Name { get; set; }
public int TotalPrice { get; set; }
public Design Design { get; set; }
public Team Team { get; set; }
public IList<Colour> Colours { get; set; }
public IList<Template> Templates { get; set; }
}
And then tried to create my Kit, it would actually create a new Colour and add the new id into the KitColours lookup table. So, when I created a Service for my KitColour and injected it into my KitService this allowed me just change the lookup table rather than the Colours table. Albeit, I had to use the KitColour navigation property instead of the Colour one. This wasn't a massive issue because the Id is all I needed.
So I moved on. Now I am working with the Design navigation property. This isn't a lookup table. Kit has one Design, but a design can be linked to many Kits. So, I get the design from the database and select the design I am interested in in my UI. The problem I have here is that it thinks that the Design is a new one and inserts it into my Designs table as a new item and updates my Kit with the new id. What it should do is get the Id from the selected design and assign that to the Kit.
Now, there are a few ways I can solve this.
First, I could change the Kit model to look like this:
public class Kit
{
public int Id { get; set; }
public string Name { get; set; }
public int TotalPrice { get; set; }
public int DesignId { get; set; }
public int TeamId { get; set; }
public IList<KitColour> Colours { get; set; }
public IList<Template> Templates { get; set; }
}
and then just specify the Id's of the Design and that would work. This can be done for design, but I have the same issue with Team also, so I would not be able to do that for that. Another option is this:
public class Kit
{
public int Id { get; set; }
public string Name { get; set; }
public int TotalPrice { get; set; }
public int DesignId { get; set; }
public Team Team { get; set; }
public IList<KitColour> Colours { get; set; }
public IList<Template> Templates { get; set; }
}
Here I keep the Team as a navigation property and just set the design id. This is my favoured choice out of the 3 because I do want to be able to select a Kit and get the attached Colours, Templates and Team. The design is negligible because it is set from a list. The problem I can see with allowing the KitService to create the Team is that the Team also has a Colours navigation property:
public class Team
{
public int Id { get; set; }
public string Name { get; set; }
public string Sport { get; set; }
public IList<TeamColour> Colours { get; set; }
public IList<Kit> Kits { get; set; }
public IList<Player> Players { get; set; }
}
So this would suffer the same issues. The third option is my least preferred. I could keep the POCO class as it is:
public class Kit
{
public int Id { get; set; }
public string Name { get; set; }
public int TotalPrice { get; set; }
public Design Design { get; set; }
public Team Team { get; set; }
public IList<KitColour> Colours { get; set; }
public IList<Template> Templates { get; set; }
}
and then in the service reference the other Services for Design, Team, KitColours and Templates. I could then use the services to call the database and pull back the items I need and reference them. Because I have pulled them from the database they would not be created, but the obvious problem here is that I am doing multiple calls to the database which (I believe) are unnecessary.
Just for clarity, here is what I think the controller would look like:
[Authorize]
[RoutePrefix("Kits")]
public class KitsController : ApiController
{
private readonly KitService service;
private readonly DesignService designService;
private readonly TeamService teamService;
/// <summary>
/// Default constructor
/// </summary>
public KitsController()
{
var unitOfWork = new UnitOfWork<DatabaseContext>();
var kitColourService = new KitColourService(unitOfWork);
var teamColourService = new TeamColourService(unitOfWork);
this.service = new KitService(unitOfWork, kitColourService);
this.designService = new DesignService(unitOfWork);
this.teamService = new TeamService(unitOfWork, teamColourService);
}
[HttpGet]
[Route("")]
/// <summary>
/// Gets a list of all the kits
/// </summary>
/// <returns>A list of kits</returns>
public async Task<IHttpActionResult> Get()
{
return Ok(await this.service.GetAllAsync("Colours", "Teamplates"));
}
[HttpGet]
[Route("")]
/// <summary>
/// Gets a kit by the id
/// </summary>
/// <param name="id">The id of the kit</param>
/// <returns>A kit</returns>
public async Task<IHttpActionResult> Get(int id)
{
return Ok(await this.service.GetAsync(id, "Colours", "Teamplates"));
}
[HttpPost]
[Route("")]
/// <summary>
/// Create a kit
/// </summary>
/// <param name="model">The kit model</param>
/// <returns>The modified kit model</returns>
public async Task<IHttpActionResult> Create(KitBindingViewModel model)
{
// If our model is invalid, return the errors
if (!ModelState.IsValid)
return BadRequest(ModelState);
// Get our design and team
var design = await this.designService.GetAsync(model.Design.Id);
var team = await this.teamService.GetAsync(model.Team.Id);
// Create our new model
var kit = new Kit()
{
Name = model.Name,
TotalPrice = model.TotalPrice,
Templates = model.Templates,
Design = design,
Team = team,
Colours = new List<KitColour>()
};
// If we have any colours
if (model.Colours != null && model.Colours.Count > 0)
{
// For each colour, insert into our lookup table
foreach (var colour in model.Colours)
kit.Colours.Add(new KitColour { ColourId = colour.Id, KitId = model.Id });
}
// Create our kit
await this.service.CreateAsync(kit, model.Colours);
// Assign our Id to our model
model.Id = kit.Id;
// Return Ok
return Ok(model);
}
}
So, apart from not using the unitofwork / service / repository design pattern, is there another way I can do this? I nicer way?
I really hope there is! I have tried to give plenty of information, but if you need anymore please ask :D
Aucun commentaire:
Enregistrer un commentaire