Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

Creates spring-cloud-gcp-autoconfigure module #276

Merged
merged 30 commits into from
Jan 11, 2018
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
f2624c8
Creates spring-cloud-gcp-autoconfigure module
joaoandremartins Dec 18, 2017
932a437
Adds spring-cloud-context to config sample
joaoandremartins Dec 18, 2017
1ec45bf
Adds spring-cloud-gcp-config to starter's dependencies
joaoandremartins Dec 18, 2017
41aea29
Adds static test dep again
joaoandremartins Dec 18, 2017
9213f28
Splits core starter into autoconfigure and core module
joaoandremartins Dec 18, 2017
c669ea9
Removes unnecessary annotation
joaoandremartins Dec 18, 2017
203d0cf
Merge branch 'master' into gh-262-autoconfigure-module
joaoandremartins Dec 18, 2017
0a4362a
Fixes stalled build
joaoandremartins Dec 19, 2017
2942c60
Adds GcpConfigAutoConfiguration back to autoconfigure's spring.factories
joaoandremartins Dec 19, 2017
13dd3d6
Merge branch 'master' into gh-262-autoconfigure-module
joaoandremartins Dec 21, 2017
8303715
EOF new line in spring.factories
joaoandremartins Dec 21, 2017
0f609e0
snicoll's code review
joaoandremartins Jan 2, 2018
2287602
fixes spring.provides
joaoandremartins Jan 2, 2018
77364d4
removes unnecessary spring.factories
joaoandremartins Jan 2, 2018
b231149
fixes build
joaoandremartins Jan 2, 2018
ac3dedb
Finish killing spring-cloud-gcp-config
joaoandremartins Jan 2, 2018
2de9f93
fix build again
joaoandremartins Jan 2, 2018
a53aef5
Merge branch 'master' into gh-262-autoconfigure-module
joaoandremartins Jan 9, 2018
902eea3
Adds the config module back in
joaoandremartins Jan 9, 2018
2b7852a
Adds GoogleConfigPropertySourceLocator to the bootstrap configurators
joaoandremartins Jan 9, 2018
5813ae8
Fixes Javadoc and other bits
joaoandremartins Jan 9, 2018
0d47060
Adds necessary spring.factories
joaoandremartins Jan 9, 2018
c5486c7
Changes import order to fix build in rare conditions
joaoandremartins Jan 9, 2018
2ccc845
Fixes indentation
joaoandremartins Jan 9, 2018
1da213a
Fix indentation
joaoandremartins Jan 9, 2018
96c58fd
New line
joaoandremartins Jan 9, 2018
2d39c0d
Merge branch 'master' into gh-262-autoconfigure-module
joaoandremartins Jan 10, 2018
7c3da59
Merge branch 'master' into gh-262-autoconfigure-module
joaoandremartins Jan 10, 2018
252a533
Changes commons-logging's scope to test
joaoandremartins Jan 10, 2018
13c45aa
Fixes issues with AppEngineCondition
joaoandremartins Jan 11, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
<module>spring-cloud-gcp-integration</module>
<module>spring-cloud-gcp-docs</module>
<module>spring-cloud-gcp-samples</module>
<module>spring-cloud-gcp-autoconfigure</module>
<module>spring-cloud-gcp-config</module>
</modules>

<properties>
Expand Down
37 changes: 37 additions & 0 deletions spring-cloud-gcp-autoconfigure/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<parent>
<artifactId>spring-cloud-gcp</artifactId>
<groupId>org.springframework.cloud</groupId>
<version>1.0.0.BUILD-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-cloud-gcp-autoconfigure</artifactId>
<name>Spring Cloud GCP Autoconfigure Module</name>

<dependencies>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-autoconfigure</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-gcp-core</artifactId>
<optional>true</optional>
</dependency>
<!-- Configuration processor -->
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-configuration-processor</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-gcp-config</artifactId>
<optional>true</optional>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,25 @@
* limitations under the License.
*/

package org.springframework.cloud.gcp.config.autoconfig;
package org.springframework.cloud.gcp.autoconfigure.config;

import org.springframework.boot.autoconfigure.AutoConfigureAfter;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.cloud.gcp.config.GcpConfigProperties;
import org.springframework.cloud.gcp.core.autoconfig.GcpContextAutoConfiguration;
import org.springframework.cloud.gcp.autoconfigure.core.GcpContextAutoConfiguration;
import org.springframework.cloud.gcp.config.GoogleConfigPropertySourceLocator;
import org.springframework.context.annotation.Configuration;

/**
* Bootstrap auto configuration for Google Cloud Runtime Configurator Starter.
*
* @author Jisha Abubaker
* @author João André Martins
*/
@Configuration
@AutoConfigureAfter(GcpContextAutoConfiguration.class)
@EnableConfigurationProperties(GcpConfigProperties.class)
@ConditionalOnClass(GoogleConfigPropertySourceLocator.class)
public class GcpConfigAutoConfiguration {

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,22 @@
* limitations under the License.
*/

package org.springframework.cloud.gcp.config;
package org.springframework.cloud.gcp.autoconfigure.config;

import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.NestedConfigurationProperty;
import org.springframework.cloud.gcp.config.GcpConfigPropertiesProvider;
import org.springframework.cloud.gcp.core.Credentials;
import org.springframework.cloud.gcp.core.GcpScope;

/**
* Configuration for {@link GoogleConfigPropertySourceLocator}.
* Configuration for {@link org.springframework.cloud.gcp.config.GoogleConfigPropertySourceLocator}.
*
* @author Jisha Abubaker
* @author João André Martins
*/
@ConfigurationProperties("spring.cloud.gcp.config")
public class GcpConfigProperties {
public class GcpConfigProperties implements GcpConfigPropertiesProvider {

private boolean enabled = true;

Expand Down Expand Up @@ -71,7 +73,7 @@ public void setTimeout(int timeout) {
this.timeout = timeout;
}

public int getTimeout() {
public int getTimeoutMillis() {
return this.timeout;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

package org.springframework.cloud.gcp.core.autoconfig;
package org.springframework.cloud.gcp.autoconfigure.core;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -41,7 +41,6 @@
import org.springframework.cloud.gcp.core.Credentials;
import org.springframework.cloud.gcp.core.DefaultGcpProjectIdProvider;
import org.springframework.cloud.gcp.core.GcpProjectIdProvider;
import org.springframework.cloud.gcp.core.GcpProperties;
import org.springframework.cloud.gcp.core.GcpScope;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@
* limitations under the License.
*/

package org.springframework.cloud.gcp.core;
package org.springframework.cloud.gcp.autoconfigure.core;

import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.NestedConfigurationProperty;
import org.springframework.cloud.gcp.core.Credentials;

/**
* @author Vinicius Carvalho
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
org.springframework.boot.autoconfigure.EnableAutoConfiguration=\
org.springframework.cloud.gcp.autoconfigure.core.GcpContextAutoConfiguration

org.springframework.cloud.bootstrap.BootstrapConfiguration=\
org.springframework.cloud.gcp.autoconfigure.config.GcpConfigAutoConfiguration,\
Copy link
Contributor

Choose a reason for hiding this comment

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

That's not BootstrapConfiguration part. This must be for the org.springframework.boot.autoconfigure.EnableAutoConfiguration property

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is correct, because it's for BootstrapConfiguration, as required by Spring Cloud Config.

org.springframework.cloud.gcp.autoconfigure.core.GcpContextAutoConfiguration
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know enough about that stuff to be positive but the fact those classes are both auto-config and bootstrap config is a smell IMO.

(nothing to do with this issue per se but I thought I'd mention it).

Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@
* limitations under the License.
*/

package org.springframework.cloud.gcp.config;
package org.springframework.cloud.gcp.autoconfigure.config;

import org.junit.After;
import org.junit.Test;

import org.springframework.boot.test.util.EnvironmentTestUtils;
import org.springframework.cloud.gcp.config.autoconfig.GcpConfigAutoConfiguration;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;

import static org.junit.Assert.assertEquals;
Expand All @@ -45,7 +44,7 @@ public void testConfigurationValueDefaultsAreAsExpected() {
GcpConfigProperties config = this.context.getBean(GcpConfigProperties.class);
assertEquals(config.getName(), null);
assertEquals(config.getProfile(), "default");
assertEquals(config.getTimeout(), 60000);
assertEquals(config.getTimeoutMillis(), 60000);
assertEquals(config.isEnabled(), true);
}

Expand All @@ -59,7 +58,7 @@ public void testConfigurationValuesAreCorrectlyLoaded() {
GcpConfigProperties config = this.context.getBean(GcpConfigProperties.class);
assertEquals(config.getName(), "myapp");
assertEquals(config.getProfile(), "prod");
assertEquals(config.getTimeout(), 120000);
assertEquals(config.getTimeoutMillis(), 120000);
assertFalse(config.isEnabled());
assertEquals(config.getProjectId(), "pariah");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

package org.springframework.cloud.gcp.core.autoconfig;
package org.springframework.cloud.gcp.autoconfigure.core;

import java.util.List;

Expand Down
34 changes: 34 additions & 0 deletions spring-cloud-gcp-config/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<parent>
<artifactId>spring-cloud-gcp</artifactId>
<groupId>org.springframework.cloud</groupId>
<version>1.0.0.BUILD-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-cloud-gcp-config</artifactId>
<name>Spring Cloud GCP Runtime Configuration Module</name>

<dependencies>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-web</artifactId>
</dependency>
<dependency>
<groupId>commons-logging</groupId>
<artifactId>commons-logging</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

<scope>test</scope>

Copy link
Contributor

Choose a reason for hiding this comment

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

+1. Can't have commons-logging as a transitive for user apps. OTOH, if we just merge this tiny module and make it depend on Spring Boot (like it feels it should), then it's probably moot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Merger of the config module is being discussed in #299 (comment)

As per my comment, I don't think it's doable, but your input is welcome!

<scope>test</scope>
</dependency>
<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-context</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-gcp-core</artifactId>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright 2017 original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.cloud.gcp.config;

import org.springframework.cloud.gcp.core.Credentials;

/**
* Provides the required properties for Spring Cloud GCP Runtime Configuration.
*
* @author João André Martins
*/
public interface GcpConfigPropertiesProvider {

/**
* Determines if Spring Cloud GCP Runtime Configuration is enabled.
*/
boolean isEnabled();

/**
* The name of the Spring Application being run.
*/
String getName();

/**
* The profile running the app. For example, "prod", "dev", etc.
*/
String getProfile();

/**
* The timeout for reaching Google Runtime Configuration API.
*/
int getTimeoutMillis();

/**
* An override to the GCP project ID in the core module.
*/
String getProjectId();

/**
* An override to the GCP credentials in the core module.
*/
Credentials getCredentials();
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,28 +70,30 @@ public class GoogleConfigPropertySourceLocator implements PropertySourceLocator

public GoogleConfigPropertySourceLocator(GcpProjectIdProvider projectIdProvider,
CredentialsProvider credentialsProvider,
GcpConfigProperties gcpConfigProperties) throws IOException {
Assert.notNull(gcpConfigProperties, "Google Config properties must not be null");
GcpConfigPropertiesProvider gcpConfigPropertiesProvider) throws IOException {
Assert.notNull(gcpConfigPropertiesProvider, "Google Config properties must not be null");

if (gcpConfigProperties.isEnabled()) {
if (gcpConfigPropertiesProvider.isEnabled()) {
Assert.notNull(credentialsProvider, "Credentials provider cannot be null");
Assert.notNull(projectIdProvider, "Project ID provider cannot be null");
this.credentials = gcpConfigProperties.getCredentials().getLocation() != null
org.springframework.cloud.gcp.core.Credentials configCredentials =
gcpConfigPropertiesProvider.getCredentials();
this.credentials = configCredentials != null && configCredentials.getLocation() != null
? GoogleCredentials.fromStream(
gcpConfigProperties.getCredentials().getLocation().getInputStream())
.createScoped(gcpConfigProperties.getCredentials().getScopes())
gcpConfigPropertiesProvider.getCredentials().getLocation().getInputStream())
.createScoped(gcpConfigPropertiesProvider.getCredentials().getScopes())
: credentialsProvider.getCredentials();
this.projectId = gcpConfigProperties.getProjectId() != null
? gcpConfigProperties.getProjectId()
this.projectId = gcpConfigPropertiesProvider.getProjectId() != null
? gcpConfigPropertiesProvider.getProjectId()
: projectIdProvider.getProjectId();
Assert.notNull(this.credentials, "Credentials must not be null");

Assert.notNull(this.projectId, "Project ID must not be null");

this.timeout = gcpConfigProperties.getTimeout();
this.name = gcpConfigProperties.getName();
this.profile = gcpConfigProperties.getProfile();
this.enabled = gcpConfigProperties.isEnabled();
this.timeout = gcpConfigPropertiesProvider.getTimeoutMillis();
this.name = gcpConfigPropertiesProvider.getName();
this.profile = gcpConfigPropertiesProvider.getProfile();
this.enabled = gcpConfigPropertiesProvider.isEnabled();
Assert.notNull(this.name, "Config name must not be null");
Assert.notNull(this.profile, "Config profile must not be null");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
org.springframework.cloud.bootstrap.BootstrapConfiguration=\
org.springframework.cloud.gcp.config.GoogleConfigPropertySourceLocator
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,25 @@

/**
* @author Jisha Abubaker
* @author João André Martins
*/
public class GoogleConfigPropertySourceLocatorTest {

private GcpConfigProperties gcpConfigProperties;
private GcpConfigPropertiesProvider gcpConfigProperties;
private Map<String, Object> expectedProperties;
private GoogleConfigPropertySourceLocator googleConfigPropertySourceLocator;
private GcpProjectIdProvider projectIdProvider;
private CredentialsProvider credentialsProvider;

@Before
public void setUp() throws Exception {
this.gcpConfigProperties = new GcpConfigProperties();
this.gcpConfigProperties.setName("test");
public void setUp() {
this.gcpConfigProperties = mock(GcpConfigPropertiesProvider.class);
when(this.gcpConfigProperties.getName()).thenReturn("test");
when(this.gcpConfigProperties.isEnabled()).thenReturn(true);
org.springframework.cloud.gcp.core.Credentials configCredentials =
mock(org.springframework.cloud.gcp.core.Credentials.class);
when(this.gcpConfigProperties.getCredentials()).thenReturn(configCredentials);
when(this.gcpConfigProperties.getProfile()).thenReturn("default");
this.expectedProperties = new HashMap<>();
this.expectedProperties.put("property-int", 10);
this.expectedProperties.put("property-bool", true);
Expand All @@ -77,7 +83,7 @@ public void locateReturnsMapPropertySource() throws Exception {

@Test
public void locateReturnsMapPropertySource_disabled() throws Exception {
this.gcpConfigProperties.setEnabled(false);
when(this.gcpConfigProperties.isEnabled()).thenReturn(false);
GoogleConfigEnvironment googleConfigEnvironment = mock(GoogleConfigEnvironment.class);
when(googleConfigEnvironment.getConfig()).thenReturn(this.expectedProperties);
this.googleConfigPropertySourceLocator = spy(new GoogleConfigPropertySourceLocator(
Expand All @@ -90,7 +96,7 @@ public void locateReturnsMapPropertySource_disabled() throws Exception {

@Test
public void disabledPropertySourceReturnsNull() throws Exception {
this.gcpConfigProperties.setEnabled(false);
when(this.gcpConfigProperties.isEnabled()).thenReturn(false);
this.googleConfigPropertySourceLocator = spy(new GoogleConfigPropertySourceLocator(
this.projectIdProvider, this.credentialsProvider, this.gcpConfigProperties));
this.googleConfigPropertySourceLocator.locate(new StandardEnvironment());
Expand All @@ -99,7 +105,7 @@ public void disabledPropertySourceReturnsNull() throws Exception {

@Test
public void disabledPropertySourceAvoidChecks() throws IOException {
this.gcpConfigProperties.setEnabled(false);
when(this.gcpConfigProperties.isEnabled()).thenReturn(false);
this.googleConfigPropertySourceLocator =
spy(new GoogleConfigPropertySourceLocator(null, null, this.gcpConfigProperties));
this.googleConfigPropertySourceLocator.locate(new StandardEnvironment());
Expand All @@ -108,7 +114,7 @@ public void disabledPropertySourceAvoidChecks() throws IOException {

@Test
public void testProjectIdInConfigProperties() throws IOException {
this.gcpConfigProperties.setProjectId("pariah");
when(this.gcpConfigProperties.getProjectId()).thenReturn("pariah");
this.googleConfigPropertySourceLocator = new GoogleConfigPropertySourceLocator(
this.projectIdProvider, this.credentialsProvider, this.gcpConfigProperties
);
Expand Down
Loading