Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CAY-2469 Allow validationQuery to be set in domain.xml #314

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mmpestorich
Copy link

I have not been able to figure out an easy way to setup a validationQuery using a standard cayenne project file. I would expect to be able to specify it along side the connection pool min and max connections.

<?xml version="1.0" encoding="utf-8"?>
<domain xmlns="http://cayenne.apache.org/schema/10/domain"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://cayenne.apache.org/schema/10/domain http://cayenne.apache.org/schema/10/domain.xsd"
    project-version="10">
   <node name="DataNode" adapter="..." factory="o.a.c.c.s.XMLPoolingDataSourceFactory" schema-update-strategy="...">
      <map-ref name="..."/>
      <data-source>
         <driver value="..."/>
         <url value="..."/>
         <login userName="..." password="..."/>
         <connectionPool min="1" max="10" validationQuery="SELECT 1 AS VALIDATE"/>
      </data-source>
   </node>
</domain>

But validationQuery is neither defined in the domain.xsd or used when creating the data source connection pool. This pull request that does that.

Pretty sure this all makes sense.... thoughts?

Not sure if this is related in anyway to CAY-1462, which was filed quite awhile ago.

@stariy95
Copy link
Member

@mmpestorich Thanks for PR! It all makes perfect sense.

However you miss a big chunk of related functionality:

  • since 4.1 runtime checks project version at loading time, so it'll fail to load new XML
  • we have Cayenne Modeler that should at least not to drop new part (and better be able to edit it)

Also it will be great to support another option available in XMLPoolingDataSource - maxQueueWaitTime.

If you wish I can help you to finish this PR.

@mmpestorich
Copy link
Author

mmpestorich commented Aug 31, 2018

@stariy95 yup, makes sense... handling the runtime version check and adding maxQueueWaitTime should be striaght forward. I can look into the modeler as well. I should have time to get suggested changes pushed up here in the next week or so.

Thanks for the feedback.

@stariy95
Copy link
Member

stariy95 commented Sep 5, 2018

Great! Will wait for it.
Meanwhile I've pushed some changes that can help you a bit.
As your schema changes seem safe we don't want to force update all projects.

@mmpestorich
Copy link
Author

Changes

  • Add maxQueueWaitTime option
  • Add validationQuery option
  • Support project versions 10 & 11
  • Allow editing these options in the modeler

Multiple Supported Project Versions

Notes

  • This will not force an upgrade from 10 but will upgrade to 11 if an older version is encountered.
  • The domain.xsd and the modelMap.xsd are currently versioned by project version.

Project Upgrades

When creating a DataChannelDescriptor or a DataMap in code (not from an xml file) its necessary to assume a default project version (in this case 10), which is more or less how things currently work if you consider that the current code essentialy just has one supported version. With these changes, the default project version will be overriden only if a supported project version greater than that is encountered while parsing a project xml file.

Project and Schema Versioning

I have to admit that I am a little unclear on what the project version is actually supposed to represent. In each xsd file, there is a project version attribute and a namespace url. The namespace url is comprised of the same value that the project version attribute is set to and in turn, the schema files are contained in a directory cooresponding to the project version they are supposed to be used with.

In following this line of reasoning, allowing multiple project versions means that a namespace can't necessarily be known before a project xml file is read, so these changes attempt to generate one from the project version once it is known. Then the VersionAwareHandler is able to properly validate the project version is both supported by the framework and compatible with the cooresponding schema namespace as specified in the project xml (which right now just means its versioned the same).

Given these existing semantics, if there is a breaking modification in one xsd file that would require a bump to the project version, then all other unmodified schema files would necessarily have to have their versions be bumped as well (or at least duplicated, bumped and placed in a new version directory).

I've tried to stay away from that, I've never been a fan of bumping versions in things that don't actually change... in doing so, I ended up with a project file that has a project version attribute set to 11 that references map files with project versions set to 10. It works but I have to admit that this is certainly not right...

I propose allowing the xsd schema files to have versions independent of the project version (i.e. a given project version would indicate compatibility with a given set of schema versions - project-version 11 uses domain.xsd version 11 and modelMap.xsd version 10). But beofre I went there, I was hoping for a little feedback... Thoughts?

@stariy95 Can you give me some more direction?

@stariy95
Copy link
Member

@mmpestorich Sorry for taking so long, get caught by other projects :)

After evaluating this once more, I think that our current way of upgrading project version (i.e. bumping all schemas versions at once) can be better, at least for now.
It is easy to implement and more importantly to maintain.
If we choose independent versions for each schema we'll end up in a strange combination of valid versions. Supporting this (e.g. testing, documenting and bug fixing) can become really hard in that case.
I don't like the idea of having multiple copies of files too, but this doesn't affect users in any way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants