refactor: separate sql stores from datasources #4381

ndr-brt · 2024-07-31T12:03:42Z

What this PR changes/adds

Separate stores and datasources configurations, making configuration easier and clearer:

edc.datasource represents the datasource configuration, that by default requires only one default datasource, but there's the possibility to define multiple of them
edc.sql.store.<name>.datasource will associate the store to a datasource, that by default will be default

this will not break the existent configuration, but a warning log will be printed to spur adopters to migrate.

Why it does that

simplify configuration

Further notes

some deserved refactoring to CommonsConnectionPoolServiceExtension
added some documentation explaining how the datasource configuration works

Linked Issue(s)

Closes #3811

Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.

...c/main/java/org/eclipse/edc/edr/store/index/SqlEndpointDataReferenceEntryIndexExtension.java

@@ -60,7 +61,7 @@

    @Override
    public void initialize(ServiceExtensionContext context) {
-        var dataSourceName = context.getConfig().getString(DATASOURCE_SETTING_NAME, DataSourceRegistry.DEFAULT_DATASOURCE);
+        var dataSourceName = DataSourceName.getDataSourceName(DATASOURCE_NAME, DATASOURCE_SETTING_NAME, context.getConfig(), context.getMonitor());


...g/eclipse/edc/connector/controlplane/store/sql/assetindex/SqlAssetIndexServiceExtension.java

@@ -61,7 +62,7 @@

    @Override
    public void initialize(ServiceExtensionContext context) {
-        var dataSourceName = context.getConfig().getString(DATASOURCE_SETTING_NAME, DataSourceRegistry.DEFAULT_DATASOURCE);
+        var dataSourceName = DataSourceName.getDataSourceName(DATASOURCE_NAME, DATASOURCE_SETTING_NAME, context.getConfig(), context.getMonitor());


...connector/controlplane/store/sql/contractdefinition/SqlContractDefinitionStoreExtension.java

@@ -61,7 +62,7 @@

    @Override
    public void initialize(ServiceExtensionContext context) {
-        var dataSourceName = context.getConfig().getString(DATASOURCE_SETTING_NAME, DataSourceRegistry.DEFAULT_DATASOURCE);
+        var dataSourceName = DataSourceName.getDataSourceName(DATASOURCE_NAME, DATASOURCE_SETTING_NAME, context.getConfig(), context.getMonitor());


...nnector/controlplane/store/sql/contractnegotiation/SqlContractNegotiationStoreExtension.java

@@ -60,7 +66,8 @@

    @Override
    public void initialize(ServiceExtensionContext context) {
-        var dataSourceName = getDataSourceName(context);
+        var dataSourceName = DataSourceName.getDataSourceName(DATASOURCE_NAME, DATASOURCE_SETTING_NAME, context.getConfig(), context.getMonitor());


...g/eclipse/edc/connector/controlplane/store/sql/policydefinition/SqlPolicyStoreExtension.java

@@ -56,7 +60,8 @@

    @Override
    public void initialize(ServiceExtensionContext context) {
-        var dataSourceName = getDataSourceName(context);
+        var dataSourceName = DataSourceName.getDataSourceName(DATASOURCE_NAME, DATASOURCE_SETTING_NAME, context.getConfig(), context.getMonitor());


...e/edc/connector/controlplane/store/sql/transferprocess/SqlTransferProcessStoreExtension.java

@@ -60,7 +64,8 @@

    @Override
    public void initialize(ServiceExtensionContext context) {
-        var dataSourceName = getDataSourceName(context);
+        var dataSourceName = DataSourceName.getDataSourceName(DATASOURCE_NAME, DATASOURCE_SETTING_NAME, context.getConfig(), context.getMonitor());


...ain/java/org/eclipse/edc/connector/dataplane/store/sql/SqlAccessTokenDataStoreExtension.java

@@ -70,7 +75,8 @@

    @Provider
    public AccessTokenDataStore dataPlaneStore(ServiceExtensionContext context) {
-        var dataSourceName = getDataSourceName(context);
+        var dataSourceName = DataSourceName.getDataSourceName(DATASOURCE_NAME, DATASOURCE_SETTING_NAME, context.getConfig(), context.getMonitor());


.../src/main/java/org/eclipse/edc/connector/dataplane/store/sql/SqlDataPlaneStoreExtension.java

@@ -69,7 +74,8 @@

    @Provider
    public DataPlaneStore dataPlaneStore(ServiceExtensionContext context) {
-        var dataSourceName = getDataSourceName(context);
+        var dataSourceName = DataSourceName.getDataSourceName(DATASOURCE_NAME, DATASOURCE_SETTING_NAME, context.getConfig(), context.getMonitor());


...g/eclipse/edc/connector/dataplane/selector/store/sql/SqlDataPlaneInstanceStoreExtension.java

@@ -70,7 +76,8 @@

    @Provider
    public DataPlaneInstanceStore dataPlaneInstanceStore(ServiceExtensionContext context) {
-        var dataSourceName = getDataSourceName(context);
+        var dataSourceName = DataSourceName.getDataSourceName(DATASOURCE_NAME, DATASOURCE_SETTING_NAME, context.getConfig(), context.getMonitor());


.../java/org/eclipse/edc/connector/policy/monitor/store/sql/SqlPolicyMonitorStoreExtension.java

@@ -60,7 +65,8 @@

    @Provider
    public PolicyMonitorStore policyMonitorStore(ServiceExtensionContext context) {
-        var dataSourceName = context.getConfig().getString(DATASOURCE_SETTING_NAME, DEFAULT_DATASOURCE);
+        var dataSourceName = DataSourceName.getDataSourceName(DATASOURCE_NAME, DATASOURCE_SETTING_NAME, context.getConfig(), context.getMonitor());


codecov-commenter · 2024-07-31T12:22:44Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 76.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 74.91%. Comparing base (7f20ba5) to head (26edc16).
Report is 371 commits behind head on main.

Files	Patch %	Lines
.../eclipse/edc/sql/configuration/DataSourceName.java	0.00%	8 Missing ⚠️
...otiation/SqlContractNegotiationStoreExtension.java	50.00%	1 Missing ⚠️
...ne/store/sql/SqlAccessTokenDataStoreExtension.java	0.00%	1 Missing ⚠️
...ataplane/store/sql/SqlDataPlaneStoreExtension.java	0.00%	1 Missing ⚠️
...itor/store/sql/SqlPolicyMonitorStoreExtension.java	0.00%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4381      +/-   ##
==========================================
+ Coverage   71.74%   74.91%   +3.16%     
==========================================
  Files         919     1072     +153     
  Lines       18457    21453    +2996     
  Branches     1037     1173     +136     
==========================================
+ Hits        13242    16071    +2829     
- Misses       4756     4859     +103     
- Partials      459      523      +64

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

refactor: separate sql stores from datasources

26edc16

ndr-brt added enhancement New feature or request refactoring Cleaning up code and dependencies labels Jul 31, 2024

ndr-brt requested a review from wolf4ood July 31, 2024 12:03

ndr-brt requested review from paullatzelsperger, bscholtes1A, jimmarino and juliapampus as code owners July 31, 2024 12:03

github-advanced-security bot found potential problems Jul 31, 2024

View reviewed changes

wolf4ood approved these changes Jul 31, 2024

View reviewed changes

ndr-brt merged commit b99f4dc into eclipse-edc:main Aug 7, 2024
25 of 27 checks passed

ndr-brt deleted the 3811-separate-stores-from-datasources branch August 7, 2024 06:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: separate sql stores from datasources #4381

refactor: separate sql stores from datasources #4381

ndr-brt commented Jul 31, 2024

codecov-commenter commented Jul 31, 2024

refactor: separate sql stores from datasources #4381

refactor: separate sql stores from datasources #4381

Conversation

ndr-brt commented Jul 31, 2024

What this PR changes/adds

Why it does that

Further notes

Linked Issue(s)

codecov-commenter commented Jul 31, 2024

Codecov Report