1

I have the following situation:

My project contains multiple entities, each one with its respective controller, service and JPA repository. All of these entities are associated with a specific company by a "companyUuid" property.

Every incoming request in my controllers will have a "user" header, which will give me the details of the User making that request, including which company he is associated with.

I need to retrieve the company associated with the user from the header and filter every subsequent query by this company, which would be essentially like adding WHERE companyUuid = ... to each query.

What I did as a solution was a generic function for creating the Specification object:

public class CompanySpecification {

public static <T> Specification<T> fromCompany(String companyUuid) {
    return (e, cq, cb) -> cb.equal(e.get("companyUuid"), companyUuid);
}}

Implemented repository as follows:

public interface ExampleRepository extends JpaRepository<Example, Long>, JpaSpecificationExecutor<Example> { }

Changed the "find" calls to include the specification:

exampleRepository.findAll(CompanySpecification.fromCompany(companyUuid), pageRequest);

Of course, this requires adding @RequestHeader to the controller functions to get the user in the header.

Although this solution works absolutely fine, it would require a lot of copy-pasting and code repetition to get it done for all routes of my @RestControllers.

Therefore, the question is: how can I do this in an elegant and clean way for all my controllers?

I have researched this quite a bit now and I came across the following conclusions:

  1. Spring JPA and Hibernate don't seem to provide a way of dynamically using a Specification to restrict all queries (reference: Automatically Add criteria on each Spring Jpa Repository call)
  2. Spring MVC HandlerInterceptor would maybe help for getting the User out of the header in each request, but it doesn't seem to fit overall since I don't use views in this project (it's just a back-end) and it can't do anything about my repository queries
  3. Spring AOP seemed like a great option to me and I gave it a go. My intention was to keep all repository calls as they were, and add the Specification to the repository call. I created the following @Aspect:
@Aspect
@Component
public class UserAspect {

    @Autowired(required=true)
    private HttpServletRequest request;

    private String user;

    @Around("execution(* com.example.repository.*Repository.*(..))")
    public Object filterQueriesByCompany(ProceedingJoinPoint jp) throws Throwable {
        Object[] arguments = jp.getArgs();
        Signature signature = jp.getSignature();

        List<Object> newArgs = new ArrayList<>();
        newArgs.add(CompanySpecification.fromCompany(user));

        return jp.proceed(newArgs.toArray());
    }

    @Before("execution(* com.example.controller.*Controller.*(..))")
    public void getUser() {
        user = request.getHeader("user");
    }
}

This would have worked perfectly, since it would require almost no modifications at all to controllers, services and repositories. Although, I had a problem with the function signature. Since I am calling findAll(Pageable p) in my Service, the signature of the function is already defined in my advice, and I can't change to the alternative version findAll(Specification sp, Pageagle p) from inside the advice.

What do you think would be the best approach in this situation?

BDL
  • 21,052
  • 22
  • 49
  • 55
RafaelCT
  • 85
  • 2
  • 6
  • 2
    Sharing `user` field between those 2 advices is clearly a bad idea since you are in a multithreaded environment. –  Nov 20 '18 at 15:34
  • @EugenCovaci yes, you are right. I wrote this code as a primary test of AOP, but will definitely take this into account when implementing the real solution. Thanks! – RafaelCT Nov 20 '18 at 16:51

2 Answers2

2

Here is an idea:

import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.springframework.data.jpa.domain.Specification;
import org.springframework.web.context.request.RequestContextHolder;
import org.springframework.web.context.request.ServletRequestAttributes;

@Aspect
public class UserAspect {

    @Around("execution(* com.example.repository.*Repository.findAll())")
    public Object filterQueriesByCompany(ProceedingJoinPoint jp) throws Throwable {
        Object target = jp.getThis();
        Method method = target.getClass().getMethod("findAll", Specification.class);
        HttpServletRequest request = ((ServletRequestAttributes) RequestContextHolder.currentRequestAttributes()).getRequest();
        return method.invoke(target, CompanySpecification.fromCompany(request.getHeader("user")));
    }

}

The above aspect intercepts the findAll() methods from repository and, instead of proceeding the call it replaces with another call to findAll(Specification) method. Notice how I get the HttpServletRequest instance.

Of course, it's a starting point not an out of the box solution.

  • I think Rafael said that the method to be called was `findAll(Specification sp, Pageagle p)`, not just `findAll(Specification sp)` like in your suggested solution. – kriegaex Dec 11 '18 at 00:57
  • Furthermore the OP wants to intercept `findAll(Pageable)`, but your pointcut intercepts `findAll()`. So there are at least two bugs in your code. No offense meant. – kriegaex Dec 11 '18 at 02:21
  • @kriegaex When I wrote the answer, the OP methods were `findAll(Specification sp)` and `findAll()` then the question was edited later. As I said, this is not a solution, it's more like a starting point. –  Dec 11 '18 at 07:20
  • I see. Yeah, moving targets from OPs changing their minds/questions are difficult to trace here when just seeing the current status of questions and answers. – kriegaex Dec 11 '18 at 07:22
  • @kriegaex A feature like editing history would be helpful. –  Dec 11 '18 at 07:24
  • I can see the editing history, but maybe you need a minimum amount of reputation for it, I do not remember. Still you have to click the "edited x hours/days ago" link in order to see it. I do not do that unless I have reason to believe an old version contains interesting information. Besides, I just looked at the editing history and the code was **not** edited before and my own edit only fixed the code formatting and syntax highlighting. – kriegaex Dec 11 '18 at 07:45
  • @kriegaex I do not remember the `Pageable` argument. Anyway, that's not the point. –  Dec 11 '18 at 07:48
1

I am not a Spring or Java EE user, but I can help you with the aspect part. I googled a bit, too, because your code snippets without imports and package names are a bit incoherent, so I cannot just copy, paste and run them. Judging from the JavaDocs for JpaRepository and JpaSpecificationExecutor, both of which you extend in your ExampleRepository, you are trying to intercept

Page<T> PagingAndSortingRepository.findAll(Pageable pageable)

(inherited by JpaRepository) and call

List<T> JpaSpecificationExecutor.findAll(Specification<T> spec, Pageable pageable)

instead, right?

So in theory we can use this knowledge in our pointcut and advice in order to be more type-safe and avoid ugly reflection tricks. The only problem here is that the intercepted call returns Page<T> while the method you want to call instead returns List<T>. The calling method surely expects the former and not the latter, unless you always use Iterable<T> which is a super-interface for both interfaces in question. Or maybe you just ignore the return value? Without you answering that question or showing how you modified your code to do this, it will be difficult to really answer your question.

So let us just assume that the returned result is either ignored or handled as Iterable. Then your pointcut/advice pair looks like this:

@Around("execution(* findAll(*)) && args(pageable) && target(exampleRepository)")
public Object filterQueriesByCompany(ProceedingJoinPoint thisJoinPoint, Pageable pageable, ExampleRepository exampleRepository) throws Throwable {
  return exampleRepository.findAll(CompanySpecification.fromCompany(user), pageable);
}

I tested it, it works. I also think it is a bit more elegant, type-safe and readable than what you tried or what was proposed by Eugen.

P.S.: Another option is to convert the list into a corresponding page manually before returning it from the aspect advice if the calling code indeed expects a page object to be returned.


Update due to follow-up question:

Eugen wrote:

For another entity, let's say Foo, the repository would be public interface FooRepository extends JpaRepository<Foo, Long>, JpaSpecificationExecutor<Foo> { }

Well, then let us just generalise the pointcut and assume that it should always target classes which extend both interfaces in question:

@Around(
  "execution(* findAll(*)) && " +
  "args(pageable) && " + 
  "target(jpaRepository) && " +
  //"within(org.springframework.data.jpa.repository.JpaRepository+) && " +
  "within(org.springframework.data.jpa.repository.JpaSpecificationExecutor+)"
)
public Object filterQueriesByCompany(ProceedingJoinPoint thisJoinPoint, Pageable pageable, JpaRepository jpaRepository) throws Throwable {
  return ((JpaSpecificationExecutor) jpaRepository)
    .findAll(CompanySpecification.fromCompany(user), pageable);
}

The part of the pointcut I commented out is optional because I am narrowing down to JpaRepository method calls already via target() parameter binding using the advice signature. The second within() should be used, however, in order to make sure the intercepted class actually also extends the second interface so we can cast and execute the other method instead without any problems.


Update 2:

As Eugen said, you can also get rid of the cast if you bind the target object to the type JpaSpecificationExecutor - but only if you do not need the JpaRepository in your advice code before that. Otherwise you would have to cast the other way. Here it seems it is not really needed, so his idea makes the solution a little more lean and expressive, indeed. Thanks for the contribution. :-)

@Around(
  "target(jpaSpecificationExecutor) && " +
  "execution(* findAll(*)) && " +
  "args(pageable) && " + 
  "within(org.springframework.data.jpa.repository.JpaRepository+)"
)
public Object filterQueriesByCompany(ProceedingJoinPoint thisJoinPoint, Pageable pageable, JpaSpecificationExecutor jpaSpecificationExecutor) throws Throwable {
  return jpaSpecificationExecutor.findAll(CompanySpecification.fromCompany(user), pageable);
}

Or alternatively, if you do not want to merge execution() with within() (a matter of taste):

@Around(
  "target(jpaSpecificationExecutor) && " +
  "execution(* org.springframework.data.jpa.repository.JpaRepository+.findAll(*)) && " +
  "args(pageable)" 
)
public Object filterQueriesByCompany(ProceedingJoinPoint thisJoinPoint, Pageable pageable, JpaSpecificationExecutor jpaSpecificationExecutor) throws Throwable {
  return jpaSpecificationExecutor.findAll(CompanySpecification.fromCompany(user), pageable);
}

Less type-safe, but also an option if you believe that the are no other classes with * findAll(Pageable) signature:

@Around("target(jpaSpecificationExecutor) && execution(* findAll(*)) && args(pageable)")
public Object filterQueriesByCompany(ProceedingJoinPoint thisJoinPoint, Pageable pageable, JpaSpecificationExecutor jpaSpecificationExecutor) throws Throwable {
  return jpaSpecificationExecutor.findAll(CompanySpecification.fromCompany(user), pageable);
}

You might notice that this suspiciously looks like my original solution for one specific sub-interface, and you are right. I recommend to be a little more strict, though, and not use the last option, even though it works in my test case and you would probably be fine with it.

Finally, I think we have covered most bases by now.

kriegaex
  • 63,017
  • 15
  • 111
  • 202
  • Unless I'm missing something, you do suppose all repositories implements `ExampleRepository`. –  Dec 11 '18 at 07:38
  • Of course I do because this is what the OP's question clearly implies. Otherwise the trick to intercept a method call from one interface and then call a method from another interface would not even work. His own `ExampleRepository` extends both of them, which is the reason why what he wants to do theoretically makes sense in the first place. – kriegaex Dec 11 '18 at 07:47
  • That's not like Spring Data works. `ExampleRepository` is the repository interface for `Example` entity only. –  Dec 11 '18 at 07:51
  • For another entity, let's say `Foo `the repository would be `public interface FooRepository extends JpaRepository, JpaSpecificationExecutor { }` –  Dec 11 '18 at 07:53
  • Interesting (implicit) question. As I said in my answer's first sentence: I am not a Spring or Java EE user. Anyway, please see my answer update and tell me if this would do for you. – kriegaex Dec 11 '18 at 09:00
  • Looks good IMO. I think you can get rid of that cast by removing `within(org.springframework.data.jpa.repository.JpaSpecificationExecutor+)` and using `target(jpaSpecificationExecutor) && within(org.springframework.data.jpa.repository.JpaRepository+)` inside the pointcut then `public Object filterQueriesByCompany(ProceedingJoinPoint thisJoinPoint, Pageable pageable, JpaSpecificationExecutor jpaSpecificationExecutor) `. After all `JpaSpecificationExecutor` is all that matters. –  Dec 11 '18 at 10:11
  • Thanks again for your feedback, I updated the answer once more with three more variants of your suggestion. – kriegaex Dec 12 '18 at 01:26