Difference between revisions of "Talk:EclipseLink/Development/AdditionalCriteria"

Latest revision as of 09:41, 28 September 2010

Requirements

It would be good to list the requirements of this feature and the use cases that we expect to solve, and examples of how these are solved. Specifically where the requirement for multiple criteria and being able to enable/disable comes from and how this is expected to be used.

i.e.

• soft deletes
• history filtering
• multi tenancy
• user based security
• others? (please list)

James.sutherland.oracle.com 13:41, 28 September 2010 (UTC)

Design details

The AdditionalCriteria maps to EclipseLink's existing descriptor additionalJoinExpression. The AdditionalCriteriaGroup and Callback and naming and enabled options are new functionality. Some insight into their requirement, usage, design and implementation in the runtime would be useful.

James.sutherland.oracle.com 19:11, 20 September 2010 (UTC)

Yes I will add details to that effect. This was merely the first stab at the metadata side of things. I will add more details concerning the core side of things next.

guy.pelletier.oracle.com 8:00, 21 September 2010 (UTC)

I still don't get how the new functionality will be supported. Currently there is a single additionalJoinExpression, having multiple criteria with names and a concept of being enabled or disable does not exist. How will this functionality be exposed to the user, there must be new runtime API. What is this?

James.sutherland.oracle.com 18:38, 22 September 2010 (UTC)

>> boolean enabled() default false;

Shouldn't the default be enabled. Using the criteria would seem to be the norm, not not using it.

James.sutherland.oracle.com 13:29, 27 September 2010 (UTC)

>> public void disableAdditionalCriteria(String name) >> public void enableAdditionalCriteria(String name) >> public void setAdditionalCriteriaParameterValue(String name, Object value)

These do not make sense. The only point of enabling/disabling is per session, you cannot globally descriptor settings on the fly. This will not work in a concurrent or multi user system. The API needs to be on Session, or IsolatedClientSession, and EntityManager, it is a runtime API, not a meta-data API.

James.sutherland.oracle.com 13:29, 27 September 2010 (UTC)

>> eclipselink.enable-additional-criteria.Employee = CompanyFilter

I would expect the naming and definition of an AdditionalCriteria to be global in scope, the same as sequencing and converters. Users who use soft deletes, history or multi-tenancy normally use them for their entire object model, so would not want to have to repeat it for every class. This also makes the runtime API simpler, you just enable/disable a single criteria,

i.e. entityManager.unwrap(JpaEntityManager.class).enableAdditionalCriteria("TenantFilter")

James.sutherland.oracle.com 13:29, 27 September 2010 (UTC)

>> eclipselink.additional-criteria-parameter.model.Employee:COMPANY = ORACLE

Just having a Session property Expression / function that using any Session property would be simpler, then you don't need a custom API. The user would just set their property on the EntityManager, similar to how they do proxy sessions currently.

i.e. entityManager.setProperty("tenant", currentTenant);

James.sutherland.oracle.com 13:29, 27 September 2010 (UTC)

Caching and query preparation

How is this going to affect caching and query preparation. This needs to be discussed. The default enabled criteria should be cacheable and prepared into the mapping and named queries and query parse cache. If a session enables or disables a specific criteria, then it will not be able to use the shared cache or any prepared query (including mapping queries). This will have a major impact on performance, so selectively enabling/disabling a criteria should be well thought out. Normally a separate ServerSession/EntityManagerFactory would be best, i.e. an admin persistence unit that removes the filters, so can cache and prepare queries.

James.sutherland.oracle.com 13:29, 27 September 2010 (UTC)

Callbacks

The callback seems quite complex, and uses reflection. Some similar to our DescriptorCustomizer and SessionCustomizer may be simpler. We could have a QueryCustomizer interface that takes a DatabaseQuery before execution and allows the user to customizes it. Or the existing SessionEventListener could be used.

James.sutherland.oracle.com 19:19, 20 September 2010 (UTC)

There is a requirement from Doug to provide this capability outside of a customizer, providing ::users with an easier configuration option.

guy.pelletier.oracle.com 8:00, 21 September 2010 (UTC)

A QueryCustomizer would still meet this requirement, and be consistent with our existing API, and not require reflection.

Another option would be a JPA event for preExecuteQuery that the user could annotate the same as any other JPA event, probably taking a JPA Query as argument, not a DatabaseQuery.

James.sutherland.oracle.com 13:13, 21 September 2010 (UTC)

Part of the reason is that I believe Doug was looking for a solution that would not require the user to implement another internal interface class. Similar to the way entity listeners work. Tying into the existing SessionEvent is great I guess if they are already using one, if not this work would require the user to create one which I think Doug wanted to avoid.

The preExecuteQuery option sounds interesting though, but sounds somewhat similar if not the same approach already proposed? You annotate a method on the callback class. Perhaps allow the additionalcriteriacallbackmethod to be defined on the entity as well?

guy.pelletier.oracle.com 10:05, 21 September 2010 (UTC)

Do we want to allow the per query(ies) filter that an additional criteria callback method currently offers, and if so would we not want to allow this filter on the additional criteria as well? Meaning we would allow multiple additional criteria to be defined. Or do we strip this per query filtering and allow only a single callback method?

guy.pelletier.oracle.com 10:05, 21 September 2010 (UTC)

I still don't see the real point of these. We already have several mechanisms to customize a query. Adding another, instead of just the metadata to support the existing ones seems odd.

• operations for delete, insert, update, read-object, read-all can be customized in SQL, or procedures, (or the query could be customized)
• default redirectors are supported for operations, delete, insert, update, read-object, read-all, query
• pre/postExecuteQuery session events

Why is it called a callback, not an event? Why is it called an additionalCriteria when it seems it can do whatever it wants to the query, not just add additional criteria. Why is it a choice between additionCriteria, doesn't seem to have anything to do with it.

James.sutherland.oracle.com 18:38, 22 September 2010 (UTC)

JPQL

What is the context for the JPQL string, i.e. alias? Additional join expressions are normally table/field level. Will support be added to our JPQL support to allow referencing database fields/tables?

James.sutherland.oracle.com 19:24, 20 September 2010 (UTC)

I think that for the first phase the JPQL fragment has to be limited to a few simple rules. Any thing beyond should be left as an enhancement. My thoughts are that for phase one the following rules should be true:

two types of fragments will be supported:

1. leftSide comparator rightSide

 -where leftSide is a mapping, (eg firstname, or address.city)
 -comparator should be limited to these:
   =   // equal
   <>  // not equal
   !=  // not equal
   <   // less than
   <=  // less than or equal
   >   // greater than
   >=  // greater than or equal
   LIKE // like
  -rightSide is a litteral or a named argument (identified by starting with ':')

2. leftSide comparison

 -where leftSide is a mapping, (eg firstname, or address.city)
 -comparison is limited to:
  IS NULL
  IS NOT NULL
  IS EMPTY 
  IS NOT EMPTY

Peter.Krogh.oracle.com

Why limit anything? Aren't we just using our existing JPQL parser? I think it can parse a where clause only, and even if it can't maybe just prepend "Select this from <class> this where " to it and parse it and get the selectionCriteria from the resulting query, so the alias "this" would need to be used in the fragment ("this.isDeleted = false"), or better ("COLUMN(this, 'IS_DELETED') = false").

James.sutherland.oracle.com 13:13, 21 September 2010 (UTC)

With lifting the JPQL limiting, this removes the need of the AdditionalCriteriaGroup so I am removing that.

guy.pelletier.oracle.com 11:52, 22 September 2010 (UTC)