Re: [eclipselink-dev] rewriting updateall queries clean-up code in tests
A few comments inline:
Andrei Ilitchev wrote:
Two questions about the suggested patch.
My understanding that Symfoware platform doesn't allow SEQUENCE to be
used as a table name,
but rather than changing all the tests that use AUTO and TABLE sequences
(and that work on all platforms other than Symfoware)
please consider re-defining default sequence table name by defining
sequence generators with magic names: "SEQ_GEN" and "SEQ_GEN_TABLE".
The sequence defined by SEQ_GEN will be used for AUTO;
the sequence defined in SEQ_GEN_TABLE will be used for table sequence in
when no CMP3_AAA_GENERATOR is defined.
In general, I agree with Andrei that we shouldn't stop testing table sequences
with the name SEQUENCE for all platforms - it has been our default for a long
time so most users will have it. The issue here will be making changes that fit
into our test framework.
In talking with Andrei, I think a better solution may be to implement a
customizer that loops through the sequences in a project and changes any table
sequence with the name "SEQUENCE" to something else. That customizer could be
enabled with a System property when running the tests on Symfoware.
Please consider instead of removing all usages of updateAll/deleteAll
queries in the tests clean up code adding a flag
supportsModifyAllQueries (or smth like that) to the platform
(or alternatively adding a persistence unit property?)
and keeping the original updateAll/deleteAll unless the platform doesn't
I am not a fan of adding a method to DatabasePlatform that will just be used for
I can see the argument that we should not alter our testing for one platform,
and that there may be some testing that occurs as a side-effect of these
clean-up methods. When I, however, look at the code we are changing, nearly all
of them exercise a DeleteAll query on a Basic field on Employee. In my opinion,
we have exercised that particular query adequately in the DeleteAll-specific
tests and the changes Dies has provided should be fine. There are a small
number these changes I have questions about, but I will be able to provide more
detail when I investigate integrating the final patch.
My vote is that we keep the majority of Dies' updates - but I am open to
arguments that go the other way.
I also noticed such queries in some other tests and wasn't sure
whether/how to fix them. What do you think?
The tests here don't seem to use UpdateAll/DeleteAll queries but the
clear method does. It uses UnitOfWork instead of EntityManager. Where
can I find the UnitOfWork API equivalent to doing a find and removing
each object in the resultset?
I am a hesitant to change this test for all platforms, but you could add an
isSymfoware() check and then do some alternate clean up. Which queries fail?
(I assume just the ones that deal with Employee)
Here is some pseudo code:
Iterator<Employee> emps = uow.readAllObjects(Employee.class).iterator()
Employee emp = emps.next();
Same as above, UpdateAll/DeleteAll with UOW in clear method. Here I
think the following tests rely on UpdateAll/Delete queries so need to be
excluded for Symfoware, would you agree?
The clear should be able to be fixed in much the same way as above.
I agree those test should be excluded for symfoware.
Same as above.
This test fails because it uses a multi-table entity. If I would rewrite
it to use an entity that is mapped to a simple table, it might pass.
Should I try, or should I just skip these type of tests on Symfoware?
Let's just skip this kind of test for Symfoware. Altering the test will reduce
our testing of multi-table.
The patch also includes a number of other issues I addressed. Could you
When we have come up with a good solution for the issue with SEQUENCE, I'll go
through the whole patch and either integrate it (with any small changes needed)
or provide feedback about bigger changes.
eclipselink-dev mailing list
eclipselink-dev mailing list