Add infrastructure for generic query augmentation [DATACMNS-293]

[spring-projects-issues]

Oliver Drotbohm opened DATACMNS-293 and commented

When working with repositories there might be use cases that involve the queries to be executed for that repository to contain some kind of global criteria. A good example for such a scenario is ACL based security to domain objects, e.g. with a global criteria causing the repository to only return domain objects created by the current user or only objects the current user is allowed to read.

A related example is working with data that has a soft-delete flag and essentially has to consider objects with that flag set to true non-existant.

Implementing these kinds of scenarios require the queries to the underlying store to be augmented with additional criterias. So we need to come up with some an SPI to prepare the query about to be executed and potentially even alter a delete(…) call into an update (for the soft-delete case).

From a more general point of view we need access to the method the query is executed for and it's associated metadata

---

Issue Links:

• SEC-2409 Spring Security / Spring Data Acl Integration
  ("is depended on by")

• DATAMONGO-1151 Introduce ability to append queries against a particular collection
  ("is depended on by")

• DATAJPA-307 Add support for soft deletes

• DATAJPA-1551 Create an API to customize query creation and query processing

116 votes, 92 watchers

[spring-projects-issues]

Neale Upstone commented

This is exactly the issue I just came looking for.

Participating in the creation and/or execution of the queries for CrudRepository methods is one part of the equation, the other is being able to add argument resolvers (such as being able to extract values from the Spring Security principle, and expose them as a named parameters, such that we could use it in JQL queries)

[spring-projects-issues]

Zoltan Altfatter commented

Would be great to have this functionality

[spring-projects-issues]

Ian Duffy commented

Any ETA on this?

[spring-projects-issues]

Pedro Vilaça commented

As this issue isn't in-progress yet, do you suggest anything as a workaround to add a filter to the queries for entities that has a specific annotation? we're working with spring security ACL and we're trying to find a way to do the filtering without need to write all the queries by hand

[spring-projects-issues]

Brian Susko commented

I'd also be interested in any recommendations on this until such case this is implemented. It seems there should be a lower-level API we can tie into to add a dynamic criteria. In the end, it would be nice to get access to the entity type also, so we can check for things like "if typeof T, add this criterion with a base interface for all entities needed. Something like that. Just my $.02, thanks

[spring-projects-issues]

Raghavendra Chary B commented

Currently we are using below as a workaround:
https://github.com/charybr/spring-data-rest-acl.

Where we use CrudRepository so that Spring security ACL PostFilter can be applied:

@RepositoryRestResource(path = "book")
public interface BookRepository extends CrudRepository<Book, Long> {

	@SuppressWarnings("unchecked")
	@Override
	@PreAuthorize("hasRole('ROLE_ADMIN') or hasPermission(#society, 'write')")
	Book save(@P("book") Book book);
	
	@Override
    @PostFilter("hasPermission(filterObject, 'read') or hasPermission(filterObject, admin)")
	Iterable<Book> findAll();	

    @PostFilter("hasPermission(filterObject, 'read') or hasPermission(filterObject, admin)")
	List<Book> findByAuthor(@Param("author") String author);

}

[spring-projects-issues]

Oliver Drotbohm commented

We'll be looking into this for the Gosling release train again. The biggest reason we haven't really gotten much farther is that we'd like to get more feedback on what we currently have. So everyone interested: there's actually a way to help us out with that:

• Grab the feature branch builds we have for the Spring Data JPA reference implementation for soft-deletes that builds on the foundation here. The version to pull in for Spring Data JPA is DATAJPA-307-SNAPSHOT, for Spring Data Commons refer to DATACMNS-293-SNAPSHOT.
• Have a look at JpaSoftDeleteQueryAugmentor and it's super types to get an impression of how to actually implement a QueryAugmentor.
• Provide feedback about the API or whether the stuff works for you at all, you'd like to see improvements etc

[spring-projects-issues]

Jordi Llach Fernandez commented

After reviewing the changes done in sprin-data-commons and spring-data-jpa here are my thoughts.

As far as I've seen QueryAugmentor's business logic is meant to be "ruled" by the values of its corresponding annotation, for instance QueryDslSoftDeleteQueryAugmentor uses SoftDelete, the same way as LockModeQueryAugmentor uses Lock

I think that is approach is right as long as we have access to the entity type in the AnnotationBasedQueryAugmentor's methods (prepareNativeQuery, prepareQuery, and prepareUpdate) through any kind of Context --> QueryContext (QueryDslJpaQueryContext,JpaQueryContext,JpaCriteriaQueryContext) and UpdateContext (QueryDslJpaUpdateContext,JpaUpdateContext).

Thus one could implement a custom augmentor that could modify the query by checking if an annotated field of the entity contains certain values.
Actually I've not reviewed all the QueryContext or UpdateContext implementations but some of them already let you access the repository interface which in turn holds the entity type.

[spring-projects-issues]

Oliver Drotbohm commented

That's very useful, thanks. Indeed annotations will definitely play an important role in augmentation as they're a nice tool to declaratively define expected augmenting behavior. That said, as you can see from the fact that QueryAugmentor doesn't say anything about annotations at all so an implementation that relies on more advanced configuration through a Spring Bean is possible, too.

The entity type is handed into the supports(…) method and will be called before any kind of augmentation invocations. The primary use case here being that the repository infrastructure can detect the need for augmentation early and potentially avoid all kinds of checks and invocations for a type that doesn't need to be augmented at all. So for query augmentations you could decide whether to match or not early on. UpdateContext actually even has the entity about to be updated, so that you could even consider special subtypes here.

Does that make sense?

[spring-projects-issues]

Jordi Llach Fernandez commented

Actually I was planning to use AnnotationBasedQueryAugmentor as a foundation class, but by looking at the implementation done in augmentQuery(...) and augmentUpdate(...) I think that these (unnecesary?) extra checks are already there, maybe I am missing something

Maybe QueryContext could also hold a reference to the entity in order to ease of use.

[spring-projects-issues]

Oliver Drotbohm commented

Just to make sure, we're on the same page here: AnnotationBasedQueryAugmentor considers repository annotations on both the type level as well as on methods. So the annotation handling implemented in it guards against an annotation type defined in the corresponding sub-type

[spring-projects-issues]

Jordi Llach Fernandez commented

My last comment about "(unnecesary?) extra checks" just tries to point out that IF supports(...) method is called BEFORE (as you described yesterday) any augmentXXX(...) method, those checks against the cache are useless, maybe I am missing something here, but to me they seem unnecesary

To make it clearer, the code I was expecting to find in AnnotationBasedQueryAugmentor is something like

      public final Q augmentQuery(Q context, MethodMetadata metadata) {
		return prepareQuery(context, cache.get(metadata.getMethod()));
	}

	public final U augmentUpdate(U update, MethodMetadata metadata) {
		return  prepareUpdate(update, cache.get( metadata.getMethod() ));
	}

About "AnnotationBasedQueryAugmentor considers repository annotations on both the type level as well as on methods" :
That's absolutely fine and I was considering to overwrite only prepareQuery(...) and/or prepareUpdate(...), and inside these ones by using the received context (QueryContext or UpdateContext) try to inspect the bean type, retrieve custom annotations defined at FIELD level and augment the query as needed, thus only using annotations handled by AnnotationBasedQueryAugmentor as a marker annotation to trigger the augmentor process. Does that make sense?

[spring-projects-issues]

Oliver Drotbohm commented

The invocation of individual methods of the QueryAugmentor are hidden behind QueryAugmentationEngine. Clients might call augmentationNeeded(…) defensively but don't necessarily have to. That's why we double check the annotation cache in the execution methods.

Also, keep in mind that the prepare… methods actually take the annotation as argument. So the cache lookup is needed anyway. We're just applying some extra safety here

[spring-projects-issues]

ken p commented

I haven't had the time to pull the aforementioned branch, so forgive me for asking prior to doing so, but will the JpaRespository#deleteInBatch(Iterable<T> entities) as well the delete all in batch be augmented when appropriate?

[spring-projects-issues]

Gazal Gaffoor commented

Any update on this issue? The code for DATAJPA-307 is ready, right? Is this issue pending on DATAMONGO-1151?