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.

Sign up for GitHub

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

Jump to bottom

making possible to override environment variables by system properties #875

Merged
merged 6 commits into from
Nov 12, 2014
Merged

making possible to override environment variables by system properties #875

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a715aae
making possible to override environment variables by system properties
nykolaslima Nov 11, 2014
ef66086
removing commons-lang and using guava
nykolaslima Nov 11, 2014
deaebc8
adding docs about overriding properties with system properties
nykolaslima Nov 11, 2014
2a26b12
typo
nykolaslima Nov 11, 2014
f037338
adding example about override environment variables
nykolaslima Nov 12, 2014
4eda003
doc improvements suggested by @Turini
nykolaslima Nov 12, 2014
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
6 changes: 6 additions & 0 deletions vraptor-core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@
<artifactId>xstream</artifactId>
<version>1.4.7</version>
</dependency>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I create a ValidationUtil class to do this?

I believe that notBlank method is something that we can use in many parts of the project.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can use guava's Strings class:

http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/base/Strings.html#isNullOrEmpty(java.lang.String)

On Tue Nov 11 2014 at 3:23:23 PM Nykolas Laurentino de Lima <
notifications@github.com> wrote:

In vraptor-core/pom.xml:

@@ -63,6 +63,12 @@
xstream
1.4.7

Should I create a ValidationUtil class to do this?

I believe that notBlank method is something that we can use in many parts
of the project.

—
Reply to this email directly or view it on GitHub
https://github.com/caelum/vraptor4/pull/875/files#r20164752.

Copy link
Contributor

Choose a reason for hiding this comment

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

By the way, I think this feature is great. And I also believe that during
development environment, the properties should be reloaded for every
request.

On Tue Nov 11 2014 at 3:26:25 PM Chico Sokol chico.sokol@gmail.com wrote:

We can use guava's Strings class:

http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/base/Strings.html#isNullOrEmpty(java.lang.String)

On Tue Nov 11 2014 at 3:23:23 PM Nykolas Laurentino de Lima <
notifications@github.com> wrote:

In vraptor-core/pom.xml:

@@ -63,6 +63,12 @@
xstream
1.4.7

Should I create a ValidationUtil class to do this?

I believe that notBlank method is something that we can use in many
parts of the project.

—
Reply to this email directly or view it on GitHub
https://github.com/caelum/vraptor4/pull/875/files#r20164752.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, use @csokol suggestion. We can't add a new jar with a lot of classes only because one method.

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 @csokol. I'll do this!

On Tuesday, November 11, 2014, csokol notifications@github.com wrote:

In vraptor-core/pom.xml:

@@ -63,6 +63,12 @@
xstream
1.4.7

We can use guava's Strings class:
http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/base/Strings.html#isNullOrEmpty(java.lang.String)
… <#1499fe486c772a65_>
On Tue Nov 11 2014 at 3:23:23 PM Nykolas Laurentino de Lima <
notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote: In
vraptor-core/pom.xml: > @@ -63,6 +63,12 @@ >
xstream > 1.4.7 >

—
Reply to this email directly or view it on GitHub
https://github.com/caelum/vraptor4/pull/875/files#r20164940.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Take it easy @garcia-jj. Already understood.

On Tuesday, November 11, 2014, Otávio Garcia notifications@github.com
wrote:

In vraptor-core/pom.xml:

@@ -63,6 +63,12 @@
xstream
1.4.7

Yes, use @csokol https://github.com/csokol suggestion. We can't add a
new jar with a lot of classes only because one method.

—
Reply to this email directly or view it on GitHub
https://github.com/caelum/vraptor4/pull/875/files#r20165131.

Copy link
Member

Choose a reason for hiding this comment

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

@csokol About reload, sounds too fair. And its not too hard to implement. May we can do this feature in next pull request. What U think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think it should be in different PR.

On Tue Nov 11 2014 at 3:31:57 PM Otávio Garcia notifications@github.com
wrote:

In vraptor-core/pom.xml:

@@ -63,6 +63,12 @@
xstream
1.4.7

@csokol https://github.com/csokol About reload, sounds too fair. And
its not too hard to implement. May we can do this feature in next pull
request. What U think?

—
Reply to this email directly or view it on GitHub
https://github.com/caelum/vraptor4/pull/875/files#r20165314.

<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
<version>3.3.2</version>
</dependency>
<!-- /mandatory -->
Copy link
Member

Choose a reason for hiding this comment

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

Please, do not add more dependencies in the core without any good reason. Please remove this and use Strings.isNullOrEmpty from Guava library, that is already exists as dependency.


<!-- [cdi] -->
Expand Down
9 changes: 8 additions & 1 deletion vraptor-core/src/main/java/br/com/caelum/vraptor/environment/DefaultEnvironment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static br.com.caelum.vraptor.environment.EnvironmentType.PRODUCTION;
import static br.com.caelum.vraptor.environment.EnvironmentType.TEST;
import static com.google.common.base.Objects.firstNonNull;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import static org.slf4j.LoggerFactory.getLogger;

import java.io.IOException;
Expand Down Expand Up @@ -91,7 +92,13 @@ public String get(String key) {
if (!has(key)) {
throw new NoSuchElementException("Key " + key + " not found in environment " + getName());
}
return properties.getProperty(key);

String systemProperty = System.getProperty(key);
if(isNotBlank(systemProperty)) {
return systemProperty;
} else {
return properties.getProperty(key);
}
}

@Override
Expand Down
10 changes: 10 additions & 0 deletions vraptor-core/src/test/java/br/com/caelum/vraptor/environment/DefaultEnvironmentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,16 @@ public void shouldGetValueIfIsSetInProperties() throws Exception {
assertEquals("development", value);
}

@Test
public void shouldGetOverridedSystemPropertyValueIfIsSet() throws IOException {
DefaultEnvironment defaultEnvironment = new DefaultEnvironment(EnvironmentType.DEVELOPMENT);
System.setProperty("env_name", "customEnv");
String value = defaultEnvironment.get("env_name");
assertEquals("customEnv", value);
//unset property to not break other tests
System.setProperty("env_name", "");
}

@Test
public void shouldTrimValueWhenEvaluatingSupport() throws Exception {
DefaultEnvironment defaultEnvironment = new DefaultEnvironment(EnvironmentType.DEVELOPMENT);
Expand Down