[RHPAM-2789] CorrelationKeyInfo table is used even without using correlation key functionality #1607
Conversation
|
This is ok to test. |
| assertTrue(log.getCorrelationKey().contains(":src.Process4")); | ||
| } finally { | ||
| System.clearProperty("org.jbpm.correlationkey.length"); | ||
| } |
There was a problem hiding this comment.
Would be probably good to correct (and retain) such a test.
There was a problem hiding this comment.
it is a faulty test in my opinion due to the way of constructing the correlation key is creating keys bigger than 70.
There was a problem hiding this comment.
Well, that is the point of the test - or should be - to check if it is trimmed in case a system property is defined.
There was a problem hiding this comment.
it is a faulty test.
persist correlation info: null
Process1
persist correlation info: 1:src.Process2:1584350223097
Process2
persist correlation info: 1:src.Process2:1584350223097:src.Process3:1584350223109
Process3
2020-03-16 10:17:03,119 [main] WARN CorrelationKey content was trimmed as it was too long (more than 70 characters)
persist correlation info: 1:src.Process2:1584350223097:src.Process3:1584350223109:src.Process4:1
2020-03-16 10:17:03,124 [main] WARN CorrelationKey content was trimmed as it was too long (more than 70 characters)
Process4
2020-03-16 10:17:03,130 [main] WARN CorrelationKey content was trimmed as it was too long (more than 70 characters)
2020-03-16 10:17:03,130 [main] WARN CorrelationKey content was trimmed as it was too long (more than 70 characters)
persist correlation info: 1:src.Process2:1584350223097:src.Process3:1584350223109:src.Process4:1
2020-03-16 10:17:03,132 [main] WARN SQL Error: 23505, SQLState: 23505
The trim is correct, the problem is that process 4 (already excceds the 70 positions) and process 5 generates the same as process 4 correlation key that is the reason it is failing now when you haven an index in database.
There was a problem hiding this comment.
than I guess we can limit it to for example to 100 and it should be fine, wdyt?
There was a problem hiding this comment.
yeah I fixed the test. just to give you the bits. The test was working because another bug in
this uses this function:
https://github.com/kiegroup/jbpm/blob/master/jbpm-persistence/jbpm-persistence-jpa/src/main/java/org/jbpm/persistence/JpaProcessPersistenceContext.java#L142
if you see it uses the externalForm of the correlation Key... the problem is that the external form is not being trimmed like the name
so it can happen that while the name is trimmed you are looking for a key not trimmed not finding the right the right one. That is the reason is a faulty test.
I did follow you recomendation and added the check
There was a problem hiding this comment.
I see... Well, the original issue was that only process instance log were affected for the reporter of the issue, so in fact CorrelationKeyInfo class change wasn't even needed in the end and was done just for consistency (which in fact wasn't consistent as you found out :) ) Good catch!
elguardian
force-pushed
the
RHPAM-2789
branch
2 times, most recently
from
33b927e
to
cd07d66
Compare
elguardian
changed the title
…elation key functionality remove select for CorrelationKeyInfo and adding unique index
|
Kudos, SonarCloud Quality Gate passed!
|
remove select for CorrelationKeyInfo and adding unique index