cayenne-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nikita Timofeev <ntimof...@objectstyle.com>
Subject Re: Cayenne 4.1M1-snapshot upgrade experience
Date Wed, 27 Sep 2017 11:48:38 GMT
I've opened pull request with fix for compatibility issue [1],
essentially it's just a cgen templates update.
Here is how updated version of data object will look like [2], note
int and Integer usage for fields intColumnNotnull and intColumnNull.

[1] https://github.com/apache/cayenne/pull/243
[2] https://github.com/stariy95/cayenne/blob/6c871b3f68294a909b5feb573e237cd0fada753e/cayenne-server/src/test/java/org/apache/cayenne/testdo/locking/auto/_SimpleLockingTestEntity.java#L28

On Tue, Sep 26, 2017 at 11:10 AM, Nikita Timofeev
<ntimofeev@objectstyle.com> wrote:
> We can also try to bring compatibility back with cgen templates. Such
> fields can be transparently boxed/unboxed in setters/getters, e.g. we
> can use Integer type instead of int inside a class, while methods
> still use int.
> I'll make some research and tests. Maybe this can be ready for 4.1.M1
> release as it seems important to me too.
>
> On Tue, Sep 26, 2017 at 8:42 AM, Andrus Adamchik <andrus@objectstyle.org> wrote:
>> Hi John,
>>
>> Thanks for sharing. This is rather important. We'll need to think of a strategy to
handle NULLs for primitives (maybe as simple as a Modeler warning ... I.e. if you expect nulls,
map a column as an object, not a primitive).
>>
>> Andrus
>>
>>> On Sep 26, 2017, at 12:22 AM, John Huss <johnthuss@gmail.com> wrote:
>>>
>>> I have partially started using 4.1M1-snapshot in production now and I did
>>> have one problem that you should be aware of.
>>>
>>> There is a difference in the data inserted between the old
>>> CayenneDataObject and the new BaseDataObject.  Previously any attributes
>>> modeled as primitive values (boolean, int) that were unset in an object and
>>> NOT mandatory (allow null) would have NULLs saved to those columns.  Now
>>> with fields being used in BaseDataObject these fields get saved with the
>>> default value for the attribute type, such as *false* for boolean and *0*
>>> for int.
>>>
>>> That in itself is probably fine, but the problem comes when using
>>> Optimistic Locking.  Rows written by the old CayenneDataObject can't be
>>> updated by the new BaseDataObject because the row was written as:
>>> attr = NULL
>>> but the new WHERE clause for the update will check for:
>>> attr = 0   (or whatever)
>>>
>>> Trying to update these rows will always throw an
>>> OptimisticLockingException.  A simple unit test can't detect this problem
>>> because it is only due to the transition from the old class to the new. But
>>> you can update a column to null with plain SQL and then see it happen.
>>>
>>> You can check if you have any attributes that could cause this problem with
>>> code like this:
>>>
>>> for (DataMap map : getRuntime().getDataDomain().getDataMaps()) {
>>>
>>> for (ObjEntity entity : map.getObjEntities()) {
>>>
>>> for (ObjAttribute attr : entity.getAttributes()) {
>>>
>>> if (attr.getJavaClass().isPrimitive() &&
>>>
>>> attr.isUsedForLocking() &&
>>>
>>> !attr.getDbAttribute().isMandatory()) {
>>>
>>> // these attributes are handled differently by the old CayenneDataObject
>>>
>>> // and the newer BaseDataObject, which can cause
>>> OptimisticLockingExceptions.
>>>
>>> System.out.println("Possible optimistic locking problem: " + entity.getName()
>>> + "." + attr.getName());
>>>
>>> }
>>>
>>> }
>>>
>>> }
>>>
>>> }
>>>
>>>
>>> On Mon, Sep 18, 2017 at 12:58 PM Andrus Adamchik <andrus@objectstyle.org>
>>> wrote:
>>>
>>>> The old MergerFactory (now called MergerTokenFactory) was attached to
>>>> DbAdapter and was not injectable. Adapter acted as the factory of
>>>> MergerFactory. So it was kind of auto-detected by the virtue of the adapter
>>>> being auto-detected. Though in a different place, it is still auto-detected
>>>> now.
>>>>
>>>> In 4.0 merge token API was split away from the core runtime. It resides in
>>>> a separate module and DbAdapter no longer has access to it. To preserve the
>>>> functionality (albeit not the API), DbSyncModule creates a map of factories
>>>> by adapter, and an injectable MergerTokenFactoryProvider will give you the
>>>> right factory for your adapter. The API is marginally harder than it was,
>>>> the functionality is the same. Should work fine with AutoAdapter too. E.g.
>>>> if there's a PostgresAdapter adapter working inside AutoAdapter, the
>>>> factory will resolve to PostgresMergerTokenFactory and so on.
>>>>
>>>> I suspect the reason why the code below caused the confusion though:
>>>>
>>>>> MergerTokenFactory factory =
>>>> runtime.getInjector().getInstance(MergerTokenFactory.class);
>>>>> assert factory instanceof MySQLMergerTokenFactory;
>>>>
>>>>
>>>> A binding for "MergerTokenFactory.class" is not the factory itself, but a
>>>> _default/failover_ factory (i.e. the factory used when per-adapter lookup
>>>> failed). Per-adapter factories are stored elsewhere in a DI map (see
>>>> DbSyncModule.java for details). And the API to get a hold of the factory
is
>>>> via MergerTokenFactoryProvider, as shown by Nikita.
>>>>
>>>> I guess one "fix" we may introduce is an extra DI "qualifier" to the
>>>> default factory binding, so that the code like yours above would simply
>>>> fail instead of giving the wrong factory:
>>>>
>>>>   binder.bind(Key.get(MergerTokenFactory.class,
>>>> "failoverFactory").to(DefaultMergerTokenFactory.class);
>>>>
>>>> In the meantime please use MergerTokenFactoryProvider API.
>>>>
>>>> Andrus
>>>>
>>>>
>>>>
>>>>> On Sep 18, 2017, at 8:01 PM, John Huss <johnthuss@gmail.com> wrote:
>>>>>
>>>>> I'm not sure what to make of your comment.  Previously the
>>>>> MergeTokenFactory was correctly auto-detected. Now it is not. I call
>>>> that a
>>>>> regression.  If it was intentional, I think it was a poor choice. I have
>>>>> worked around the issue, but I recommend fixing it before this 4.1 gets
>>>>> released.
>>>>>
>>>>> On Thu, Sep 14, 2017 at 4:22 AM Nikita Timofeev <
>>>> ntimofeev@objectstyle.com>
>>>>> wrote:
>>>>>
>>>>>> Ok, I think I found what is it :) See this task [1], if you don't
want
>>>>>> to directly inject MergerTokenFactory you can use
>>>>>> MergerTokenFactoryProvider in your code like this (note that you
will
>>>>>> need DbAdapter object):
>>>>>>
>>>>>> @Inject
>>>>>> MergerTokenFactoryProvider factoryProvider;
>>>>>> // ...
>>>>>> MergerTokenFactory factory = factoryProvider.get(adapter);
>>>>>>
>>>>>> [1] https://issues.apache.org/jira/browse/CAY-2116
>>>>>>
>>>>>> On Wed, Sep 13, 2017 at 5:13 PM, John Huss <johnthuss@gmail.com>
wrote:
>>>>>>> On Wed, Sep 13, 2017 at 3:27 AM Nikita Timofeev <
>>>>>> ntimofeev@objectstyle.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi John,
>>>>>>>>
>>>>>>>> Where exactly do you see this detection failure, is it Modeler
DB
>>>>>>>> migration tool?
>>>>>>>>
>>>>>>>
>>>>>>> This is in tomcat after the ServerRuntime is created from the
builder.
>>>>>>> Here's a minimal example:
>>>>>>>
>>>>>>> ServerRuntime runtime = ServerRuntime.builder()
>>>>>>>
>>>>>>> .addConfigs("cayenne-MyProject.xml").build();
>>>>>>>
>>>>>>>
>>>>>>> MergerTokenFactory factory = runtime
>>>>>>> .getInjector().getInstance(MergerTokenFactory.class);
>>>>>>>
>>>>>>> assert factory instanceof MySQLMergerTokenFactory;
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> And how do you define your data source (via AutoAdapter or
>>>>>> MySQLAdapter)?
>>>>>>>>
>>>>>>>
>>>>>>> I have custom DataSourceFactory specified in the cayenne project,
but
>>>> no
>>>>>>> override of the adapter (for MySQL), so it is using AutoAdapter.
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Sep 12, 2017 at 11:45 PM, John Huss <johnthuss@gmail.com>
>>>>>> wrote:
>>>>>>>>> Also, the correct MergerTokenFactory for my DbAdapter
is not longer
>>>>>>>>> automatically detected (both for Postgres and MySQL).
 I had to add
>>>>>> this
>>>>>>>> to
>>>>>>>>> my ServerRuntime module to fix it.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>> binder.bind(MergerTokenFactory.class).to(PostgresMergerTokenFactory.class);
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Thu, Aug 17, 2017 at 9:21 AM John Huss <johnthuss@gmail.com>
>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> I just got done upgrading several projects to use
the latest from
>>>>>>>>>> master. I am very excited about the memory usage
improvements in
>>>> this
>>>>>>>>>> version.
>>>>>>>>>>
>>>>>>>>>> I wanted to relay my experience along with some of
the problems I
>>>>>> had in
>>>>>>>>>> case it would be helpful to others or allow this
process to be
>>>>>> smoothed
>>>>>>>>>> out.  We were previously on a version of master from
about two years
>>>>>>>> back.
>>>>>>>>>> I think most of these issues are fairly recent however.
>>>>>>>>>>
>>>>>>>>>> - Can't build without running the tests (mvn clean
install
>>>>>>>>>> -Dmaven.test.skip=true) at least once successfully
due to missing
>>>>>> maven
>>>>>>>>>> dependency for cayenne.project (which is the second
thing built). I
>>>>>>>> don't
>>>>>>>>>> know why this depends on the tests.
>>>>>>>>>>
>>>>>>>>>> - Can't successfully run tests without a newer Java
1.8 SDK. I had a
>>>>>> JDK
>>>>>>>>>> 1.8.0 from 2014 (the first release?) that didn't
work (mockito
>>>>>> errors).
>>>>>>>>>> Upgrading to the newest JDK fixed the problem.
>>>>>>>>>>
>>>>>>>>>> - new ServerRuntime(...) no longer works because
my custom DbAdapter
>>>>>>>>>> subclass would not get the new ValueObjectTypeRegistry
parameter
>>>>>>>> injected
>>>>>>>>>> automatically. Switching to ServerRuntime.builder()
fixed the
>>>>>> problem.
>>>>>>>>>>
>>>>>>>>>> - CayenneFilter chooses a different name for the
DataDomain when you
>>>>>>>> load
>>>>>>>>>> multiple projects now (is the name the constant "cayenne"?).
I use
>>>>>> this
>>>>>>>>>> name in a lot of places to set DB connection information
so I don't
>>>>>>>> want to
>>>>>>>>>> change it. Unfortunately CayenneFilter does not provide
a way to
>>>>>>>> override
>>>>>>>>>> the DataDomain's name, so I had to just stop using
CayenneFilter.
>>>>>> Using
>>>>>>>>>> runtime.getDataDomain.setName("abc") doesn't work
because the DB has
>>>>>>>>>> already been connected to at that point. CayenneFilter
should take
>>>> an
>>>>>>>>>> init-param to set the name instead.
>>>>>>>>>>
>>>>>>>>>> - Cayenne project .xml files need to be upgraded
by running the
>>>>>> Modeler
>>>>>>>>>> and re-saving them first.
>>>>>>>>>>
>>>>>>>>>> - I had previously set the PK attribute's generation
method to
>>>>>> Generated
>>>>>>>>>> for me (using Postgres). Since Postgres didn't used
to support this
>>>>>> with
>>>>>>>>>> Cayenne, this used to fall back to the default behavior
>>>>>>>>>> "Cayenne-generated". Now that this IS supported everything
fails
>>>>>> since
>>>>>>>> my
>>>>>>>>>> database isn't set up to do this yet. I switched
these back to the
>>>>>>>> default
>>>>>>>>>> "Cayenne-generated" method of generating PKs. Not
a Cayenne problem,
>>>>>> but
>>>>>>>>>> something to be aware of.
>>>>>>>>>>
>>>>>>>>>> - The SQL logger's name has changed from CommonsJdbcEventLogger
to
>>>>>> just
>>>>>>>>>> JdbcEventLogger, so I had to update places where
I was changing the
>>>>>>>> logging
>>>>>>>>>> level (mostly in properties files).
>>>>>>>>>>
>>>>>>>>>> John
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Best regards,
>>>>>>>> Nikita Timofeev
>>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best regards,
>>>>>> Nikita Timofeev
>>>>>>
>>>>
>>>>
>>
>
>
>
> --
> Best regards,
> Nikita Timofeev



-- 
Best regards,
Nikita Timofeev

Mime
View raw message