[jira] [Commented] (OPENJPA-2668) CriteriaQuery instances should not change their state based on Query instances created from them

2019-04-15 Thread Michael Wiles (JIRA) 


[ 
https://issues.apache.org/jira/browse/OPENJPA-2668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16817984#comment-16817984
 ] 

Michael Wiles commented on OPENJPA-2668:


added github project (reused an old one actually)... 
[https://github.com/michaelwiles/openjpa-spring-data-bug] with 
[test|https://github.com/michaelwiles/openjpa-spring-data-bug/blob/master/src/test/java/com/afrozaar/bug/openjpa/OpenJPA_2668_Test.java]

> CriteriaQuery instances should not change their state based on Query 
> instances created from them
> 
>
> Key: OPENJPA-2668
> URL: https://issues.apache.org/jira/browse/OPENJPA-2668
> Project: OpenJPA
>  Issue Type: Bug
>  Components: jpa
>Affects Versions: 2.4.1
>Reporter: Oliver Drotbohm
>Assignee: Mark Struberg
>Priority: Major
>
> JPA has a two-step, programatic query creation process: first, you create a 
> {{CriteriaQuery}} to build up the general query structure, add constraints on 
> it etc. Then you take that instance and create a {{(Typed)Query}} instance 
> from it using the currently available {{EntityManager}} and bind parameters 
> to it.
> According to [reports we got for Spring Data 
> JPA|https://jira.spring.io/browse/DATAJPA-969], the latter step changes the 
> state of the {{CriteriaQuery}} instance, so that subsequent creations of 
> {{Query}} instances from it (and subsequent parameter bindings in turn) don't 
> actually get applied correctly. 
> Even if the subsequent creation and parameter binding got applied, that 
> change of state in {{CriteriaQuery}} instances is problematic in concurrent 
> access scenarios as the bindings might override each other partially.
> Generally speaking I'd recommend to keep the {{CriteriaQuery}} instances 
> immutable with regards to the creation of {{Query}} instances from them and 
> the subsequent handling of those {{Query}} instances.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (OPENJPA-2668) CriteriaQuery instances should not change their state based on Query instances created from them

2019-04-15 Thread Michael Wiles (JIRA) 


[ 
https://issues.apache.org/jira/browse/OPENJPA-2668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16817979#comment-16817979
 ] 

Michael Wiles commented on OPENJPA-2668:


So having had another go at this I've made some progress...

The issue stems fundamentally from how the CriteriaQuery is cached by Spring 
Data and then it is changed when openjpa hydrates an "In" where clause.

If you look at the code in 
[Expressions.java|https://github.com/apache/openjpa/blob/9f26ed29bf31b5c8ab68c5257d42e8c88765cf9b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/Expressions.java#L1436]
 you'll notice how the Expressions$In class modifies the _exps property. This 
property btw comes out of the CriteriaQuery so changes this class changes the 
CriteriaQuery.

Now as far as I can make out, this is the only expression that is changing the 
CriteriaQuery. 

So I suspect if we simply clone the Expressions.In class and then operate on 
that one, it should work. 

So something like adding:
{code:java}
private Expressions.In copy() {
In in = new Expressions.In<>(this.e);
in._exps.addAll(this._exps);
return in;
}
{code}

And then after renaming the current toKernelExpression we make this method:


{code:java}
 @Override
org.apache.openjpa.kernel.exps.Expression 
toKernelExpression(ExpressionFactory factory, CriteriaQueryImpl q) {
return copy().toKernelExpression0(factory, q);
}
{code}



> CriteriaQuery instances should not change their state based on Query 
> instances created from them
> 
>
> Key: OPENJPA-2668
> URL: https://issues.apache.org/jira/browse/OPENJPA-2668
> Project: OpenJPA
>  Issue Type: Bug
>  Components: jpa
>Affects Versions: 2.4.1
>Reporter: Oliver Drotbohm
>Assignee: Mark Struberg
>Priority: Major
>
> JPA has a two-step, programatic query creation process: first, you create a 
> {{CriteriaQuery}} to build up the general query structure, add constraints on 
> it etc. Then you take that instance and create a {{(Typed)Query}} instance 
> from it using the currently available {{EntityManager}} and bind parameters 
> to it.
> According to [reports we got for Spring Data 
> JPA|https://jira.spring.io/browse/DATAJPA-969], the latter step changes the 
> state of the {{CriteriaQuery}} instance, so that subsequent creations of 
> {{Query}} instances from it (and subsequent parameter bindings in turn) don't 
> actually get applied correctly. 
> Even if the subsequent creation and parameter binding got applied, that 
> change of state in {{CriteriaQuery}} instances is problematic in concurrent 
> access scenarios as the bindings might override each other partially.
> Generally speaking I'd recommend to keep the {{CriteriaQuery}} instances 
> immutable with regards to the creation of {{Query}} instances from them and 
> the subsequent handling of those {{Query}} instances.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Issue with using an 'In' filter with Spring Data Jpa

2019-04-15 Thread Michael Wiles 
Hi All

This ticket  was logged against
spring data jpa, where a subsequent ticket
 was logged against
openjpa...

Having a vested interest in spring data plying nice with openjpa I'm taken
it upon myself to look into this...

Now it is true that CriteriaQuery is cached by spring data - which I think
is reasonable.

They suggested they stop caching altogether - which I don't think is a good
idea.

So what happens on the openjpa side is that when the CriteriaQuery is built
into an expression it changes the original CriteriaQuery - it hydrates it
with the actual parameters (parameter values) and so when the query is
called again it does not work.

The following simple test fails as the second time the query is run the
parameters will still by Michael and James.

public void Members() {
repository.save(new Member(2L, "Michael"));
repository.save(new Member(3L, "James"));

assertThat(repository.findByNameIn(Lists.newArrayList("Michael",
"James"))).hasSize(2);

assertThat(repository.findByNameIn(Lists.newArrayList("Michael"))).hasSize(1);
}


The important thing to note however is that, if I'm not mistaken this
CriteriaQuery modification *only *occurs in the In expression filter. see
https://github.com/apache/openjpa/blob/master/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/Expressions.java
line
1436.

You'll notice the _exps property is cleared and then added to... but this
is the only time, that I can see, where the _exps property is edited in
situ.

Without majorly changing how things work, I've changed it so that it acts
on a copy of the Expresssion and not on the original expression and it
works.

Added the following copy method to Expressions$In

private Expressions.In copy() {
In in = new Expressions.In<>(this.e);
in._exps.addAll(this._exps);
return in;
}

Renamed the current toKernelExpression method to differentiate (added a 0)
and then changed the current one to:

@Override
org.apache.openjpa.kernel.exps.Expression
toKernelExpression(ExpressionFactory factory, CriteriaQueryImpl q) {
return copy().toKernelExpression0(factory, q);
}

And now my test passes.

Can I request this code be looked at and potentially included? I'm fairly
convinced there's no reason why it shouldn't work.

Should I create a pull request?

Thanks

-- 
see my blog:
http://analysis102.blogspot.com
http://audiblethoughts.blogspot.com
http://outsideofficehours.blogspot.com