Reduce/remove repetitive business logic checks in ASP.NET Core

Question

Is there a way to reduce/remove constant duplication of user access checks (or some other checks) in a business layer?

Let's consider a following example: simple CRUD application with one entity BlogPost:

public class BlogPost
{
    public int Id { get; set; }
    public string Title { get; set; }
    public string Body { get; set; }

    public int AuthorId { get; set; }
}

In PUT/DELETE requests before modifying or deleting entity I need to make a check whether the user that's making request is author of BlogPost, so he is permitted to delete/edit it.

So both in UpdateBlogPost and DeleteBlogPost of imaginary BlogPostService I'll have to write something like this:

var blogPostInDb = _blogPostRepository.GetBlogPost();

if(blogPostInDb == null)
{
    // throw exception or do whatever is needed
}

if(blogPostInDb.AuthorId != _currentUser.Id)
{
   // throw exception etc...
}

This kind of code will be the same for both Updateand Delete methods as well as other methods that may be added in future and the same for all entities.

Is there any way to reduce or completely remove such duplication?

I thought this over and came up with following solutions, but they don't satisfy me fully.

First solution

Using filters. We can create some custom filters like [EnsureEntityExists] and [EnsureUserCanManageEntity] but this way we're spreading some of business logic in our API layer and it's not flexible enough since we need to create such filter for every entity. Perhaps some kind of generic filter can be made using reflection.

Also there is another problem with this approach, let's say we've made such filter that's checking our rules. We're fetching entity from db, doing checks, throwing exceptions and all that stuff and letting controller method execute. BUT in service layer we need to fetch entity again, so we're making two roundtrips to db. Maybe I'm overthinking this problem and that's fine to make 2 roundtrips, taking into account that fact that caching can be applied.

Second solution

Since I'm using CQRS (or at least some kind of it) I have MediatR library and I can make use of Pipeline Behaviors and even pass fetched entity further into pipeline via mutating TRequest (which I don't wanna do). This solution requires some common interface for all requests to be able to retrieve id of the entity. The roundtrip problem also applicable here too.

public interface IBlogPostAccess
{
    public int Id { get; set; }
}

public class ChangeBlogPostCommand: IRequest, IBlogPostAccess
{
    // ...
}

public class DeleteBlogPostCommand: IRequest, IBlogPostAccess
{
    // ...
}

public class BlogPostAccessBehavior : IPipelineBehavior where TRequest : IBlogPostAccess
{
    // all nessesary stuff injected via DI
   
    public BlogPostAccessBehavior()
    {
    }

    public async Task Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate next)
    {
        var blogPostInDb = _blogPostRepository.GetBlogPost(request.Id);
        if(blogPostInDb == null)
        {
            // throw exception or do whatever is needed
        }

        if(blogPostInDb.AuthorId != _currentUser.Id)
        {
           // throw exception etc...
        }

        return await next();
    }
}

Third solution

Create something like request context service. In a very simplified way it will be a dictionary that will be persisted across request where we can store data (in this case our BlogPost that we've fetched in filter/pipeline). This seems lame and recalls me a ViewBag in ASP.NET MVC.

Fourth solution

It's more enhancement than a solution, but we can use GuardClause or extension methods to reduce nesting of if statements.

Again, maybe I'm overthinking this problem or it's not a problem at all or that's a design issue. Any help, thoughts appreciated.

user_16224930 · Answer

If you are concerned about many database calls you could try caching the returned objects per request with something like LazyCache https://github.com/alastairtree/LazyCache

I would not recommend caching across requests...

For code organization, I would recommend extracting the authorization logic into a separate method and calling that method each request. Benefit is that if the logic changes then only need to updated it in one place.

For example something like this:

bool canEdit(userId){
    var user = getUserByUserId(userId);
    if(user.IsAdmin) return true;

    //depending on where this method lives might have access to blogpost here
    if(_blogPost.AuthorId == userId) return true;

    return false;
}

Reduce/remove repetitive business logic checks in ASP.NET Core

1 Answers