C#•2y ago

✅ Dependency Injection question

Should Repositories be Scoped or Transient? They are injected into MediatR request handlers

70 Replies

off the top of my head, I'd say scoped but that really means nothing the only REAL answer is "depends on context" what do your repositories do, and how are they used?

Jimmacle•2y ago

mediatr handlers are transient, idk how it handles scoped services

barcodeOP•2y ago

accessing data from dbcontext using ef core, for example getcategorybyid

JakenVeina•2y ago

if they're pulling scoped services, they should maybe also be scoped unless they maintain no state, and you don't expect them to be used more than once per scope in which case transient prolly makes more sense

barcodeOP•2y ago

ty that's what I thought to use, some course on clean arhitecture I watched just uses scoped with no explanation why

Jimmacle•2y ago

mediatr handlers shouldn't be scoped, and that's not the default if you use its DI integration also, is your EF context your repository or are you doing that bad thing where you're wrapping EF in repositories

JakenVeina•2y ago

there is nothing wrong with wrapping EF with a repository layer the wrongess is in HOW you implement a repository layer

barcodeOP•2y ago

public interface IReviewRepository : IAsyncRepository<Review>
{
    Task<IReadOnlyList<Review>> GetByCenterId(uint id, int page, int count);
    Task<IReadOnlyList<Review>> GetByUserId(uint id, int page, int count);
}

public interface IReviewRepository : IAsyncRepository<Review>
{
    Task<IReadOnlyList<Review>> GetByCenterId(uint id, int page, int count);
    Task<IReadOnlyList<Review>> GetByUserId(uint id, int page, int count);
}

public class ReviewRepository : BaseRepository<Review>, IReviewRepository
{
    public ReviewRepository(GymRadarDbContext dbContext) : base(dbContext)
    {
    }

    public async Task<IReadOnlyList<Review>> GetByCenterId(uint id, int page, int count)
    {
        return await DbContext.Reviews
            .Where(x => x.CenterId == id)
            .OrderBy(x => x.CreatedAt)
            .Skip(page * count)
            .Take(count)
            .ToListAsync();
    }

    public async Task<IReadOnlyList<Review>> GetByUserId(uint id, int page, int count)
    {
        return await DbContext.Reviews
            .Where(x => x.UserId == id)
            .OrderBy(x => x.CreatedAt)
            .Skip(page * count)
            .Take(count)
            .ToListAsync();
    }
}

public class ReviewRepository : BaseRepository<Review>, IReviewRepository
{
    public ReviewRepository(GymRadarDbContext dbContext) : base(dbContext)
    {
    }

    public async Task<IReadOnlyList<Review>> GetByCenterId(uint id, int page, int count)
    {
        return await DbContext.Reviews
            .Where(x => x.CenterId == id)
            .OrderBy(x => x.CreatedAt)
            .Skip(page * count)
            .Take(count)
            .ToListAsync();
    }

    public async Task<IReadOnlyList<Review>> GetByUserId(uint id, int page, int count)
    {
        return await DbContext.Reviews
            .Where(x => x.UserId == id)
            .OrderBy(x => x.CreatedAt)
            .Skip(page * count)
            .Take(count)
            .ToListAsync();
    }
}

barcodeOP•2y ago

barcodeOP•2y ago

I just dependency inject it into repository implementation

Jimmacle•2y ago

and how do you use the repository in your handlers?

barcodeOP•2y ago

Jimmacle•2y ago

seems pretty similar to what i've been doing, except i don't use a repository or mapper all that gets done in the handler, because that's what the handler is for

JakenVeina•2y ago

what's on BaseRepository

barcodeOP•2y ago

i am trying to learn clean arhitecture that's why there is a mapper implementation of default interface for deleting updating entities

JakenVeina•2y ago

and IAsyncRepository<> yes, what

barcodeOP•2y ago

sec

JakenVeina•2y ago

that definitely smells like the repository anit-pattern

jcotton42•2y ago

transient can use scoped

JakenVeina•2y ago

just, with the assumption that they shouldn't be used in root scope

barcodeOP•2y ago

public class BaseRepository<T> : IAsyncRepository<T> where T : class
{
    protected readonly GymRadarDbContext DbContext;

    public BaseRepository(GymRadarDbContext dbContext)
    {
        DbContext = dbContext;
    }

    public virtual async Task<T?> GetByIdAsync(uint id)
    {
        T? t = await DbContext.Set<T>().FindAsync(id);
        return t;
        
    }

    public async Task<IReadOnlyList<T>> ListAllAsync()
    {
        return await DbContext.Set<T>().ToListAsync();
    }

    public async Task<IReadOnlyList<T>> GetPagedResponseAsync(int page, int size)
    {
        return await DbContext.Set<T>().Skip((page - 1) * size).Take(size).AsNoTracking().ToListAsync();
    }

    public async Task<T> AddAsync(T entity)
    {
        await DbContext.Set<T>().AddAsync(entity);
        await DbContext.SaveChangesAsync();

        return entity;
    }

    public async Task UpdateAsync(T entity)
    {
        DbContext.Entry(entity).State = EntityState.Modified;
        await DbContext.SaveChangesAsync();
    }

    public async Task DeleteAsync(T entity)
    {
        DbContext.Set<T>().Remove(entity);
        await DbContext.SaveChangesAsync();
    }
}

public class BaseRepository<T> : IAsyncRepository<T> where T : class
{
    protected readonly GymRadarDbContext DbContext;

    public BaseRepository(GymRadarDbContext dbContext)
    {
        DbContext = dbContext;
    }

    public virtual async Task<T?> GetByIdAsync(uint id)
    {
        T? t = await DbContext.Set<T>().FindAsync(id);
        return t;
        
    }

    public async Task<IReadOnlyList<T>> ListAllAsync()
    {
        return await DbContext.Set<T>().ToListAsync();
    }

    public async Task<IReadOnlyList<T>> GetPagedResponseAsync(int page, int size)
    {
        return await DbContext.Set<T>().Skip((page - 1) * size).Take(size).AsNoTracking().ToListAsync();
    }

    public async Task<T> AddAsync(T entity)
    {
        await DbContext.Set<T>().AddAsync(entity);
        await DbContext.SaveChangesAsync();

        return entity;
    }

    public async Task UpdateAsync(T entity)
    {
        DbContext.Entry(entity).State = EntityState.Modified;
        await DbContext.SaveChangesAsync();
    }

    public async Task DeleteAsync(T entity)
    {
        DbContext.Set<T>().Remove(entity);
        await DbContext.SaveChangesAsync();
    }
}

public interface IAsyncRepository<T> where T : class
{
    Task<T?> GetByIdAsync(uint id);
    Task<IReadOnlyList<T>> ListAllAsync();
    Task<T> AddAsync(T entity);
    Task UpdateAsync(T entity);
    Task DeleteAsync(T entity);
    Task<IReadOnlyList<T>> GetPagedResponseAsync(int page, int size);
}

public interface IAsyncRepository<T> where T : class
{
    Task<T?> GetByIdAsync(uint id);
    Task<IReadOnlyList<T>> ListAllAsync();
    Task<T> AddAsync(T entity);
    Task UpdateAsync(T entity);
    Task DeleteAsync(T entity);
    Task<IReadOnlyList<T>> GetPagedResponseAsync(int page, int size);
}

JakenVeina•2y ago

thaaaaaaaaaaaaaaaaaaaaaaaaat is the repository anti-pattern

Jimmacle•2y ago

yeah that gives you nothing over just injecting a dbcontext it already does all that

barcodeOP•2y ago

what does repository anti pattern mean

jcotton42•2y ago

(also AddAsync on the DbSet is rarely useful) you've just created a kneecapped version of DbSet

Jimmacle•2y ago

EF core already implements the repository pattern with dbsets

jcotton42•2y ago

severely kneecapped

JakenVeina•2y ago

for basically no gain the ONLY gain you've got here is that it's slightly more abstractable than a DbSet slightly

Jimmacle•2y ago

i mean, if you're switching ORMs deep into a project you probably have other issues

barcodeOP•2y ago

so the slight abstraction is not worth it?

jcotton42•2y ago

Jimmacle•2y ago

lemme go grab one of my handlers

jcotton42•2y ago

you're throwing away all the power of EF

JakenVeina•2y ago

abstraction of a DbSet/DbContext isn't for the purpose of being able to change out your data layer, that's a fool's errand. It's pretty much exclusively for testing

jcotton42•2y ago

(and I'd argue for testing you should use an actual database)

JakenVeina•2y ago

depends on the testing you want

Jimmacle•2y ago

public class GetProjectDetailsQueryHandler : IRequestHandler<GetProjectDetailsQuery, Result<ProjectDetailsDto>>
{
    private readonly PortalDbContext _db;

    public GetProjectDetailsQueryHandler(PortalDbContext db)
    {
        _db = db;
    }
    
    public async Task<Result<ProjectDetailsDto>> Handle(GetProjectDetailsQuery request, CancellationToken cancellationToken)
    {
        var details = await _db.Projects.AsNoTracking()
            .Where(p => p.Number == request.Number)
            .Select(p => new ProjectDetailsDto(
                p.Number,
                p.Status,
                p.Title,
                p.Description,
                p.BillingType,
                p.CreatedOn,
                p.StartedOn,
                p.ClosedOn,
                p.FlexProjectNumbers,
                p.OracleOrderNumbers,
                p.LabWork,
                p.QuotePriceDollars,
                p.QuotePriceOther,
                p.Handler,
                p.Contact,
                _db.Expenses.Where(x => x.ProjectNumber == p.Number && x.BillToCustomer).Sum(x => x.AmountUsd),
                _db.Expenses.Where(x => x.ProjectNumber == p.Number).Sum(x => x.AmountUsd)))
            .FirstOrDefaultAsync(cancellationToken);

        return details ?? Result<ProjectDetailsDto>.Fail("Project not found.");
    }
}

public class GetProjectDetailsQueryHandler : IRequestHandler<GetProjectDetailsQuery, Result<ProjectDetailsDto>>
{
    private readonly PortalDbContext _db;

    public GetProjectDetailsQueryHandler(PortalDbContext db)
    {
        _db = db;
    }
    
    public async Task<Result<ProjectDetailsDto>> Handle(GetProjectDetailsQuery request, CancellationToken cancellationToken)
    {
        var details = await _db.Projects.AsNoTracking()
            .Where(p => p.Number == request.Number)
            .Select(p => new ProjectDetailsDto(
                p.Number,
                p.Status,
                p.Title,
                p.Description,
                p.BillingType,
                p.CreatedOn,
                p.StartedOn,
                p.ClosedOn,
                p.FlexProjectNumbers,
                p.OracleOrderNumbers,
                p.LabWork,
                p.QuotePriceDollars,
                p.QuotePriceOther,
                p.Handler,
                p.Contact,
                _db.Expenses.Where(x => x.ProjectNumber == p.Number && x.BillToCustomer).Sum(x => x.AmountUsd),
                _db.Expenses.Where(x => x.ProjectNumber == p.Number).Sum(x => x.AmountUsd)))
            .FirstOrDefaultAsync(cancellationToken);

        return details ?? Result<ProjectDetailsDto>.Fail("Project not found.");
    }
}

barcodeOP•2y ago

so instead of repositories I should just directly access dbcontext?

Jimmacle•2y ago

that's all it has to be

JakenVeina•2y ago

anyway, yeah, there can be value in using a repository as a testing seam, or just as a mechanism for organization

Jimmacle•2y ago

i implemented paging/ordering/etc as extension methods on IQueryable

JakenVeina•2y ago

like, keeping data query logic in one place

jcotton42•2y ago

since you're projecting I don't think that AsNoTracking is doing anything

Jimmacle•2y ago

iirc it gets upset if you try to do certain types of projection with a tracking query

JakenVeina•2y ago

unless you have a specific reason not to don't just make a repository layer cause it's "clean" or whatever for me, the big anti-pattern is having that RepositoryBase<T> that forces you into assumptions about how your entities work

Jimmacle•2y ago

i need to clean up that project in general though, i botched the idea of VSA and didn't get any of the benefits

JakenVeina•2y ago

implement queries as you need them, don't assume all entites should be inserted/deleted/updated/etc. the exact same way

barcodeOP•2y ago

i wondered why the course creator just threw away iqueryable and opted out for a load everything into list method

jcotton42•2y ago

incompetence to be brutally honest

JakenVeina•2y ago

that's a bit of an oversimplification, but.... yeah

barcodeOP•2y ago

i am gonna restructure the project to remove that abstraction

JakenVeina•2y ago

I mean, if your instructor is expecting it.... maybe not

barcodeOP•2y ago

no i am just following a course to learn splitting an api into multiple projects

JakenVeina•2y ago

oh, so this is like an online thing? like a video series or blog series?

barcodeOP•2y ago

JakenVeina•2y ago

ugh where is that from?

barcodeOP•2y ago

https://app.pluralsight.com/library/courses/asp-dot-net-core-6-clean-architecture/discussion

JakenVeina•2y ago

UGH I thought pluralsight was supposed to be reputable well, anyway

barcodeOP•2y ago

i found few holes

JakenVeina•2y ago

there IS something to be said about the "cleanness" of keeping data-access logic separate from business logic but don't add an abstraction/cleanliness layer when you're not getting demonstrable benefit and don't hamstring yourself into implementing all things a certain way, by making interfaces/base classes for everything use those tools sparingly, when you have an appropriate reason that's just as much a part of "Clean" architecture as anything else

barcodeOP•2y ago

before I just put all files into single project and it got messy after 1 month

JakenVeina•2y ago

don't over-implement and clean architecture is a fine thing to learn to address that problem the topic of repositories over EF, in particular, is contentious around here

barcodeOP•2y ago

I don't see myself switching from EF but if I wanted to switch to Dapper lets say, wouldn't the repositories help?

jcotton42•2y ago

so there's two ways this would go

JakenVeina•2y ago

maybe as in not that having repositories would make the switch easier more that repositories might fit a little better over top of Dapper

jcotton42•2y ago

1) you constrain yourself to features common to both EF and Dapper, switching won't be that hard, but your dev experience will suck and you'll be underutilizing EF 2) you use EF to it's potential, and a switch to Dapper involves an absolute assload of work

JakenVeina•2y ago

Dapper isn't really itself a repository library, as I understand

barcodeOP•2y ago

ty for all info i will make use of it

JakenVeina•2y ago

Florian Voß•2y ago

it is reputable I'm sure there is a reason why the author is not implementing it following all best practices up front. The is something called "cognitive overload" and you wanna explain one concept at a time. The way these courses often work is they show how to get it working at all first, and then how to improve it step by step following best practices @ReactiveVeina

Gaming

Programming

✅ Dependency Injection question