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

[OSGi] Support the Data Service Specification for JDBC™ Technology #133

Merged
merged 2 commits into from Oct 15, 2019

Conversation

laeubi
Copy link

@laeubi laeubi commented Feb 28, 2019

Currently users are enforced to write their own code or using third party wrappers to use MariaDB together with OSGi JDBC/JPA Service,

This PR adds support for the OSGi Data Service Specification for JDBC™ Technology
This allows users of OSGi to use the MariaDB driver in a generic fashion. This is also a perquisite for using MariaDB together with the OSGi JPA Specification.

Since this part of the code is only activated inside an OSGi-Framework for all other users this change is completely transparent.


@Override
public MariaDbDataSource createDataSource(Properties props) throws SQLException {
MariaDbDataSource source = new MariaDbDataSource();
Copy link
Collaborator

@rusher rusher Mar 12, 2019

Choose a reason for hiding this comment

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

Implementation must distinguished basic implementation (MariaDbDataSource) and Connection pooling implementation (MariaDbPoolDataSource) if according properties JDBC_INITIAL_POOL_SIZE, JDBC_MAX_IDLE_TIME, JDBC_MAX_POOL_SIZE, JDBC_MAX_STATEMENTS, JDBC_MIN_POOL_SIZE, JDBC_PROPERTY_CYCLE are set

Copy link
Author

Choose a reason for hiding this comment

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

One Problem I noticed: MariaDbPoolDataSource does not implements ConnectionPoolDataSource but DataSourceFactory#createConnectionPoolDataSource needs one.
On the other hand MariaDbDataSource DOES implement ConnectionPoolDataSource even though it seems it is not really one.

Copy link
Author

Choose a reason for hiding this comment

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

Maybe it was meant that MariaDbPoolDataSource extends MariaDbDataSource and implements ConnectionPoolDataSource?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, that has to be corrected in 2.5. Issue created: https://jira.mariadb.org/browse/CONJ-692

Copy link
Author

Choose a reason for hiding this comment

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

So would it be possible to use the implementation as-is (even if not 100% correct in the sense of connection pooling) and change it when CONJ-692 is fixed?

Dictionary<String, Object> properties = new Hashtable<>();
properties.put(DataSourceFactory.OSGI_JDBC_DRIVER_CLASS, Driver.class.getName());
properties.put(DataSourceFactory.OSGI_JDBC_DRIVER_NAME, "JDBC driver for MariaDB and MySQL");
properties.put(DataSourceFactory.OSGI_JDBC_DRIVER_VERSION, Version.majorVersion + "." + Version.minorVersion);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Version.version would be better (like '2.4.1-SNAPSHOT', compare to '2.4' )

Copy link
Author

Choose a reason for hiding this comment

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

Changed to Version.version

public void start(BundleContext context) throws Exception {
Dictionary<String, Object> properties = new Hashtable<>();
properties.put(DataSourceFactory.OSGI_JDBC_DRIVER_CLASS, Driver.class.getName());
properties.put(DataSourceFactory.OSGI_JDBC_DRIVER_NAME, "JDBC driver for MariaDB and MySQL");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This correspond to MariaDbDatabaseMetaData.DRIVER_NAME

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the hint, I have changed this

@laeubi
Copy link
Author

laeubi commented Apr 8, 2019

How can we proceed on this?

@rusher
Copy link
Collaborator

rusher commented Oct 10, 2019

@laeubi

Hi, remains the point commented before and similar to other open source projects, the MariaDB Foundation needs to have shared ownership of all code that is included in the MariaDB distribution. The easiest way to achieve this is by submitting your code under the BSD-new license. (The other alternative is to sign the code contribution agreement which can be found here: https://mariadb.com/kb/en/mariadb/mca/)

Please indicate in a comment below that you are contributing your new code of the whole pull request under the BSD-new license or that you have filled out the contribution agreement and sent it.

Thanks,
diego

@laeubi
Copy link
Author

laeubi commented Oct 10, 2019

I'm contributing the new code of the whole pull request under the BSD-new license, is there any template I need to add to the changed file?

@rusher rusher merged commit 1d83166 into mariadb-corporation:master Oct 15, 2019
@laeubi laeubi deleted the support-osgi-jdbc-service branch October 15, 2019 15:11
<Import-Package>
javax.naming,javax.management,javax.net;resolution:=optional,javax.net.ssl;resolution:=optional,javax.sql,javax.transaction.xa;resolution:=optional,org.slf4j;resolution:=optional
</Import-Package>
<Import-Package>org.osgi.framework,javax.naming,javax.management,javax.net;resolution:=optional,javax.net.ssl;resolution:=optional,javax.sql,org.osgi.service.jdbc;resolution:=optional,javax.transaction.xa;resolution:=optional,org.slf4j;resolution:=optional</Import-Package>

Choose a reason for hiding this comment

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

I think you missed javax.security in this list. It prevents the driver from making SSL connections from an OSGi environment.

Copy link
Author

Choose a reason for hiding this comment

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

I just added what was required for the jdbc service, if you think more packages are required I think you can simply open a new PR as this one is already closed.

@laeubi
Copy link
Author

laeubi commented Dec 28, 2022

@rusher I just noticed that it seems the code for supporting this OSGi specification was removed from maria-db is there a specific reason? Could it be added back again?

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