Use NoCredentialsProvider when firestore emulator enabled. #555
Conversation
...re/src/main/java/com/google/cloud/spring/autoconfigure/firestore/GcpFirestoreProperties.java
Show resolved
Hide resolved
| ApplicationContextRunner contextRunner = new ApplicationContextRunner() | ||
| .withPropertyValues( | ||
| "spring.cloud.gcp.firestore.projectId=test-project", | ||
| "spring.cloud.gcp.firestore.emulator.enabled=true", |
There was a problem hiding this comment.
How about also a test where the property is not set?
| "spring.cloud.gcp.firestore.host-port=localhost:8080" | ||
| ).run(context -> { | ||
| CredentialsProvider defaultCredentialsProvider = context.getBean(CredentialsProvider.class); | ||
| assertThat(defaultCredentialsProvider).isNotInstanceOf(NoCredentialsProvider.class); |
There was a problem hiding this comment.
I feel like we're missing the assertion on the actual Firestore credentials provider which this PR is all about.
I think you'll need to add a package-private getter for the credentialsProvider in GcpFirestoreAutoConfiguration.
There was a problem hiding this comment.
You're right, added assertion on the new commit to make sure the GcpFirestoreAutoConfiguration did change to NoCredentialsProvider when emulator enabled.
…lsProvider when emulator enabled.
| CredentialsProvider defaultCredentialsProvider = context.getBean(CredentialsProvider.class); | ||
| assertThat(defaultCredentialsProvider).isNotInstanceOf(DefaultCredentialsProvider.class); |
There was a problem hiding this comment.
Two minor testing things:
- It would be good to validate that the mocked
CredentialsProvideris returned. One way to do that is to use.withBean(() -> /* handle to mock */ )instead of the current.withUserConfiguration(GcpFirestoreEmulatorAutoConfigurationTests.TestConfiguration.class). - Could you also test
firestoreAutoConfiguration.getCredentialsProvider()here?
There was a problem hiding this comment.
@elefeint thanks for the review! pushed new commit, would you mind have a look and let me know what you think ?
…reAutoConfiguration.getCredentialsProvider() return the mocked bean.
|
Kudos, SonarCloud Quality Gate passed! |
|
@meltsufin -- Could you take another look again when you get a chance? |
Bumps [spring-test](https://github.com/spring-projects/spring-framework) from 5.3.20 to 5.3.21. - [Release notes](https://github.com/spring-projects/spring-framework/releases) - [Commits](spring-projects/spring-framework@v5.3.20...v5.3.21) --- updated-dependencies: - dependency-name: org.springframework:spring-test dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Fix GH-501