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

feat: Migrate spanner spring data r2dbc #1971

Merged

Conversation

prash-mi
Copy link
Contributor

feat: Migrate spanner spring data r2dbc

dependabot bot and others added 30 commits September 23, 2021 10:08
Bumps [junit-bom](https://github.com/junit-team/junit5) from 5.8.0 to 5.8.1.
- [Release notes](https://github.com/junit-team/junit5/releases)
- [Commits](junit-team/junit5@r5.8.0...r5.8.1)

---
updated-dependencies:
- dependency-name: org.junit:junit-bom
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…orm#443)

Bumps [spring-boot-starter-parent](https://github.com/spring-projects/spring-boot) from 2.5.4 to 2.5.5.
- [Release notes](https://github.com/spring-projects/spring-boot/releases)
- [Commits](spring-projects/spring-boot@v2.5.4...v2.5.5)

---
updated-dependencies:
- dependency-name: org.springframework.boot:spring-boot-starter-parent
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…atform#441)

Bumps `r2dbc.version` from 0.8.5.RELEASE to 0.8.6.RELEASE.

Updates `r2dbc-spi` from 0.8.5.RELEASE to 0.8.6.RELEASE
- [Release notes](https://github.com/r2dbc/r2dbc-spi/releases)
- [Changelog](https://github.com/r2dbc/r2dbc-spi/blob/v0.8.6.RELEASE/CHANGELOG)
- [Commits](r2dbc/r2dbc-spi@v0.8.5.RELEASE...v0.8.6.RELEASE)

Updates `r2dbc-spi-test` from 0.8.5.RELEASE to 0.8.6.RELEASE
- [Release notes](https://github.com/r2dbc/r2dbc-spi/releases)
- [Changelog](https://github.com/r2dbc/r2dbc-spi/blob/v0.8.6.RELEASE/CHANGELOG)
- [Commits](r2dbc/r2dbc-spi@v0.8.5.RELEASE...v0.8.6.RELEASE)

---
updated-dependencies:
- dependency-name: io.r2dbc:r2dbc-spi
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: io.r2dbc:r2dbc-spi-test
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [libraries-bom](https://github.com/GoogleCloudPlatform/cloud-opensource-java) from 23.0.0 to 23.1.0.
- [Release notes](https://github.com/GoogleCloudPlatform/cloud-opensource-java/releases)
- [Changelog](https://github.com/GoogleCloudPlatform/cloud-opensource-java/blob/master/CHANGELOG.md)
- [Commits](https://github.com/GoogleCloudPlatform/cloud-opensource-java/commits)

---
updated-dependencies:
- dependency-name: com.google.cloud:libraries-bom
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [reactor-bom](https://github.com/reactor/reactor) from 2020.0.11 to 2020.0.12.
- [Release notes](https://github.com/reactor/reactor/releases)
- [Commits](reactor/reactor@2020.0.11...2020.0.12)

---
updated-dependencies:
- dependency-name: io.projectreactor:reactor-bom
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [spring-data-r2dbc](https://github.com/spring-projects/spring-data-build) from 1.3.5 to 1.3.6.
- [Release notes](https://github.com/spring-projects/spring-data-build/releases)
- [Commits](https://github.com/spring-projects/spring-data-build/commits)

---
updated-dependencies:
- dependency-name: org.springframework.data:spring-data-r2dbc
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… style check. (GoogleCloudPlatform#451)

Fix links in CONTRIBUTING to reflect adoc style.
Also added a line mentioning google-java-format plugin for reformating.
…orm#453)

Bumps [spring-boot-starter-parent](https://github.com/spring-projects/spring-boot) from 2.5.5 to 2.5.6.
- [Release notes](https://github.com/spring-projects/spring-boot/releases)
- [Commits](spring-projects/spring-boot@v2.5.5...v2.5.6)

---
updated-dependencies:
- dependency-name: org.springframework.boot:spring-boot-starter-parent
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [libraries-bom](https://github.com/GoogleCloudPlatform/cloud-opensource-java) from 23.1.0 to 24.0.0.
- [Release notes](https://github.com/GoogleCloudPlatform/cloud-opensource-java/releases)
- [Changelog](https://github.com/GoogleCloudPlatform/cloud-opensource-java/blob/master/CHANGELOG.md)
- [Commits](https://github.com/GoogleCloudPlatform/cloud-opensource-java/commits)

---
updated-dependencies:
- dependency-name: com.google.cloud:libraries-bom
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [checkstyle](https://github.com/checkstyle/checkstyle) from 9.0.1 to 9.1.
- [Release notes](https://github.com/checkstyle/checkstyle/releases)
- [Commits](checkstyle/checkstyle@checkstyle-9.0.1...checkstyle-9.1)

---
updated-dependencies:
- dependency-name: com.puppycrawl.tools:checkstyle
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [reactor-bom](https://github.com/reactor/reactor) from 2020.0.12 to 2020.0.13.
- [Release notes](https://github.com/reactor/reactor/releases)
- [Commits](reactor/reactor@2020.0.12...2020.0.13)

---
updated-dependencies:
- dependency-name: io.projectreactor:reactor-bom
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [spring-data-r2dbc](https://github.com/spring-projects/spring-data-build) from 1.3.6 to 1.4.0.
- [Release notes](https://github.com/spring-projects/spring-data-build/releases)
- [Commits](https://github.com/spring-projects/spring-data-build/commits)

---
updated-dependencies:
- dependency-name: org.springframework.data:spring-data-r2dbc
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…tform#703)

This fixes compiling error on mvn test-compile phase.
Added TODO comment to check on test functionality later.
… -Dit.pubsub-emulator settings. (GoogleCloudPlatform#708)

This is PR brings PubSubMessageChannelBinderEmulatorIntegrationTests to junit 5 to avoid mis-match with the abstract test class it inherit from. This test has been running with all modules's integration tests regardless of the -Dit.pubsub-emulator settings.
This is step 1 of GoogleCloudPlatform#707 and should unblock and reveal any further issues on it test for other modules.
Update sample pom to match spring boot 2.6.
Adding Spanner JSON Type support
JSON support is present in Client library since 6.12.0 release.
This PR intend to add JSON support to our repo.
Note that ARRAY support is not included and is intended as a follow-up PR to this. (created issue to track GoogleCloudPlatform#458)

The major changes include:

- Add a JsonWrapper class.
- Add binder and decoder.
- Add String to JsonWrapper for read.
- Add to r2dbc sample and spring data r2dbc sample
- README docs
- register JsonWrapper.class as simple type in spring data dialect.
- register converters to/from Map in spring data dialect as default converters.
…rm#713)

This is a temporarily fix to let datastore basic sample work with spring boot 2.6.
circular reference is on class org.springframework.shell.result.ResultHandlerConfig (link to error)

Suspect cause:
This sample uses spring-shell for cmd line interaction. Spring shell seems to be a project relative slow in updating. We are already on the newest version 2.0.1.RELEASE which has dependencies on spring boot 1.5.8.
Bumps [spring-test](https://github.com/spring-projects/spring-framework) from 5.3.12 to 5.3.13.
- [Release notes](https://github.com/spring-projects/spring-framework/releases)
- [Commits](spring-projects/spring-framework@v5.3.12...v5.3.13)

---
updated-dependencies:
- dependency-name: org.springframework:spring-test
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…orm#461)

Bumps [spring-boot-starter-parent](https://github.com/spring-projects/spring-boot) from 2.5.6 to 2.6.0.
- [Release notes](https://github.com/spring-projects/spring-boot/releases)
- [Commits](spring-projects/spring-boot@v2.5.6...v2.6.0)

---
updated-dependencies:
- dependency-name: org.springframework.boot:spring-boot-starter-parent
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [mockito-core](https://github.com/mockito/mockito) from 4.0.0 to 4.1.0.
- [Release notes](https://github.com/mockito/mockito/releases)
- [Commits](mockito/mockito@v4.0.0...v4.1.0)

---
updated-dependencies:
- dependency-name: org.mockito:mockito-core
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…CloudPlatform#730)

This is step 2 and fixes GoogleCloudPlatform#707

There are 2 fixes in this PR:

Let PubSubEmulator extend ParameterResolver to inject the emulator port number into the instance. This is left-over from task 1 upgrading to junit5 test, solves the java.lang.NullPointerException: target error caused by no port number provided.
After above change, tests fail due to a recent change in spring-cloud-stream logic.
when checking if context already has an ObjectMapper bean, calls getBeansOfType(ObjectMapper.class), which has an internal call to assertBeanFactoryActive()
Test setup fails with error: “org.springframework.context.support.GenericApplicationContexthas not been refreshed yet”.
Fix by refresh the context.
Both fixes are within the test setup.
<parent>
<artifactId>spring-cloud-gcp</artifactId>
<groupId>com.google.cloud</groupId>
<version>3.5.4-SNAPSHOT</version><!-- {x-version-update:spring-cloud-gcp:current} -->
Copy link
Contributor

Choose a reason for hiding this comment

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

Reverting a minor maven upgrade in #1997
also fyi, we released v3.5.4 last week (June 28) and 3.x is on 3.5.5-SNAPSHOT now.
Perhaps this is leading to your CI failures.

Suggested change
<version>3.5.4-SNAPSHOT</version><!-- {x-version-update:spring-cloud-gcp:current} -->
<version>3.5.5-SNAPSHOT</version><!-- {x-version-update:spring-cloud-gcp:current} -->

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @zhumin8 , this was likely the issue. It got resolved after merging with the upstream and then bumping up the version for these two new modules

Copy link
Member

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

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

A few final polish comments.

@@ -0,0 +1,52 @@
# Cloud Spanner Spring Data R2DBC
Copy link
Member

Choose a reason for hiding this comment

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

You can omit this file, since all of it is being cleaned up anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure. removed it.

Comment on lines 23 to 32
<dependency>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-r2dbc</artifactId>
<version>1.5.13</version>
</dependency>
<dependency>
<groupId>io.r2dbc</groupId>
<artifactId>r2dbc-spi</artifactId>
<version>0.9.0.RELEASE</version>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

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

Aren't these already transitively included via spring-cloud-spanner-spring-data-r2dbc?

Also, I'm surprised we don't have to include spring-boot-starter-data-r2dbc, even though we mention it in the documentation. Are docs incorrect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes the transitive dependencies are not required to be declared here. I have removed those.

Also @meltsufin I noticed that other modules are having a starter's module mentioned in the documentation , so on the similar lines I have added a new module spring-cloud-gcp-starter-spanner-r2dbc under starters and have referred it @ spanner-spring-data-r2dbc.adoc . Please let me know if it looks good

Copy link
Member

Choose a reason for hiding this comment

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

The starters are there to integrate with auto-configuration. Since we don't have auto-configuration for this module yet, a starter doesn't make sense.
That being said, we should add auto-configuration in the future. Please add it to the list of tasks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure @meltsufin , I have removed the starter's module and have mentioned spring-cloud-spanner-spring-data-r2dbc artifact in the adoc

Copy link
Member

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

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

Minor polish.

<name>Spring Framework on Google Cloud Code Sample - Spanner-r2dbc</name>

<properties>
<sonar.skip>true</sonar.skip>
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It came from the original repo, have removed it. Lets see if it changes anything in the sonar report


<properties>
<spring-data-r2dbc.version>1.5.13</spring-data-r2dbc.version>
<spring-test.version>5.3.28</spring-test.version>
Copy link
Member

Choose a reason for hiding this comment

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

unused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed it

<dependency>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-r2dbc</artifactId>
<version>${spring-data-r2dbc.version}</version>
Copy link
Member

Choose a reason for hiding this comment

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

Do you really need this version here? I believe the version will already be provided by spring boot bom we import in the parent pom.xml.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, tested. It's getting the version from the BOM. removed it from here

<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>cloud-spanner-r2dbc</artifactId>
<version>${cloud-spanner-r2dbc.version}</version>
Copy link
Member

Choose a reason for hiding this comment

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

Can we manage this dependency version in the parent pom.xml?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, done

Copy link
Member

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

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

2 more small things

* Support for Reactive Transactions.
* Schema and data initialization utilities.

See the https://docs.spring.io/spring-data/r2dbc/docs/1.0.x/reference/html/#reference[Spring Data R2DBC documentation] for more information on how to use Spring Data R2DBC.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this would be a more durable link:

Suggested change
See the https://docs.spring.io/spring-data/r2dbc/docs/1.0.x/reference/html/#reference[Spring Data R2DBC documentation] for more information on how to use Spring Data R2DBC.
See the https://docs.spring.io/spring-data/r2dbc/docs/current/reference/html/[Spring Data R2DBC documentation] for more information on how to use Spring Data R2DBC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

pom.xml Outdated
@@ -42,6 +42,7 @@
<spring-cloud-dependencies.version>2021.0.7</spring-cloud-dependencies.version>
<spring-cloud-gcp-dependencies.version>${project.parent.version}</spring-cloud-gcp-dependencies.version>
<spring-boot-dependencies.version>2.7.13</spring-boot-dependencies.version>
<cloud-spanner-r2dbc.version>1.2.2</cloud-spanner-r2dbc.version>
Copy link
Member

Choose a reason for hiding this comment

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

Where is this used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was earlier used by spring-cloud-spanner-spring-data-r2dbc, not it's unused, so I have removed it

prash-mi added 2 commits July 20, 2023 15:05
#Minor refactor

#Minor refactor in the POM

#removed spanner r2dbc starter and updated spanner-spring-data-r2dbc.adoc

#address review comments - cleanup

#add spring-cloud-gcp-starter-spanner-r2dbc

#addressed review comments - general refactor

#version upgrade
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

83.3% 83.3% Coverage
0.0% 0.0% Duplication

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

Successfully merging this pull request may close these issues.