You should not try to pass on any extra data just for the sake of performance. Besides, usng decorators, you can't change the contract. Instead you should allow that user entity to be cached and this should typically be the responsibility of the repository implementation. With Entity Framework this is actually rather straightforward. You can call DbSet.Find(id)
and EF will first look up the entity in the cache. This prevents unneeded round trips to the database. I do this all the time.
So the only thing you have to do is add a Find(key)
or GetById
method to your repository that maps to EF's Find(key)
method and you're done.
Furthermore, I agree with Pete. Decorators should be primarily for cross-cutting concerns. Adding other things in decorators can be okay sometimes, but you seem to split up the core business logic over both the handler and its decorators. Writing the file to disk be longs to the core logic. You might be conserned about adhering to the Single Responsibility, but it seems to me that your splitting a single responsibility out over multiple classes. That doesn't mean that your command handlers should be big. As Pete said, you probably want to extract this to a service and inject this service into the handler.
Validating the authorization is a cross-cutting concern, so having this in a decorator seems okay, but there are a few problems with your current implementation. First of all, doing it like this will cause you to have many non-generic decorators, which leads to a lot of maintenance. Besides, you silently skip the execution if the user is unauthorized which is typically not what you want.
Instead of silently skipping, consider throwing an exception and prevent the user from being able to call this functionality under normal circumstances. This means that if an exception is thrown, there's either a bug in your code, or the user is hacking your system. Silently skipping without throwing an exception can make it much harder to find bugs.
Another thing is that you might want to consider is trying to implement this authorization logic as generic decorator. For instance have a generc authorization decorator or validation decorator. This might not always be possible, but you might be able to mark commands with an attribute. For instance, in the system I'm currently working on we mark our commands like this:
[PermittedRole(Role.LabManagement)]
We have a AuthorizationVerifierCommandHandlerDecorator<TCommand>
that checks the attributes of the command being executed and verifies whether the current user is allowed to execute that command.
UPDATE
Here's an example of what I think your UpdateUserProfileImageCommandHandler
could look like:
public class UpdateUserProfileImageCommandHandler
: ICommandHandler<UpdateUserProfileImageCommand>
{
private readonly IFileService fileService;
public UpdateUserProfileImageCommandHandler(IFileService fileService)
{
this.fileService = fileService;
}
public void Handle(UpdateUserProfileImageCommand command)
{
var user = userRespository.GetById(command.Id);
this.fileService.DeleteFile(user.ProfileImageUri);
command.ImageUri = this.fileService.Upload(generatedUri, command.Image);
user.ProfileImageUri = command.ImageUri;
}
}