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 <johnth...@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 <and...@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 <johnth...@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 <
>> ntimof...@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 <johnth...@gmail.com> wrote:
>>>>> On Wed, Sep 13, 2017 at 3:27 AM Nikita Timofeev <
>>>> ntimof...@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 <johnth...@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 <johnth...@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
>>>> 
>> 
>> 

Reply via email to