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 >>>> >> >>