Skip to content

GEODE-4146: fix XmlEntity matching for JdbcConnectorService#1215

Merged
kirklund merged 2 commits intoapache:developfrom
kirklund:GEODE-4146-fix-JdbcClusterConfigDistributedTest
Jan 3, 2018
Merged

GEODE-4146: fix XmlEntity matching for JdbcConnectorService#1215
kirklund merged 2 commits intoapache:developfrom
kirklund:GEODE-4146-fix-JdbcClusterConfigDistributedTest

Conversation

@kirklund
Copy link
Copy Markdown
Contributor

@kirklund kirklund commented Jan 2, 2018

The JdbcConnectorService usage of XmlEntity was resulting in multiple jdbc connector service elements being written to cluster config. This then results in already exists exceptions during startup.

JdbcClusterConfigDistributedTest reproduced the problem we saw during manual testing and verifies the fix.

JdbcClusterConfigDistributedTest exposes the failure and confirms the fix.
@nreich
Copy link
Copy Markdown

nreich commented Jan 2, 2018

Fails rat (missing copyright notice) and spotless: once fixed, I approve of the changes.

XmlEntity createXmlEntity(FunctionContext<?> context) {
return new XmlEntity(createCacheProvider(context), CACHE, PREFIX, NAMESPACE,
CONNECTION_SERVICE.getTypeName());
CONNECTION_SERVICE.getTypeName(), NAME, CONNECTION_SERVICE.getTypeName());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is confusing, are we saying the parent and child types are same?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. XmlEntity has a flawed and confusing design. Someone needs to redo that class.

@kirklund kirklund merged commit c537f55 into apache:develop Jan 3, 2018
@kirklund kirklund deleted the GEODE-4146-fix-JdbcClusterConfigDistributedTest branch January 3, 2018 19:04
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.

4 participants