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

Add environment variable support for user-mgt configuration #2343

Open
wants to merge 1 commit into
base: 4.5.x
Choose a base branch
from
Open

Add environment variable support for user-mgt configuration #2343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 28 additions & 0 deletions core/org.wso2.carbon.base/src/main/java/org/wso2/carbon/base/CarbonBaseUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,32 @@ public static String getCarbonHome() {
}
return carbonHome;
}

public static String replaceSystemProperty(String text) {
Copy link

Choose a reason for hiding this comment

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

Add a java doc comment.

int indexOfStartingChars = -1;
int indexOfClosingBrace;

// The following condition deals with properties.
Copy link

Choose a reason for hiding this comment

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

Multiline comments should be used if the comment spans over a single line.

// Properties are specified as ${system.property},
// and are assumed to be System properties
Copy link

Choose a reason for hiding this comment

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

Each sentence must conclude with an apostrophe.

while (indexOfStartingChars < text.indexOf("${")
&& (indexOfStartingChars = text.indexOf("${")) != -1
&& (indexOfClosingBrace = text.indexOf('}')) != -1) {
Copy link

Choose a reason for hiding this comment

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

Can't we validate the pattern of "${system.property}" using a regex?
Also, what is the reason to use a while loop? Can't we replace it with an if condition after matching the pattern?

String sysProp = text.substring(indexOfStartingChars + 2,
Copy link

Choose a reason for hiding this comment

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

This can also be handled using a regex pattern which will improve the readability.

indexOfClosingBrace);
String propValue = System.getProperty(sysProp);
Copy link

Choose a reason for hiding this comment

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

Line 142-146 can be simplified using Optionals.
E.g. Optional.orElseGet()

if (propValue == null) {
propValue = System.getenv(sysProp);
}
if (propValue != null) {
text = text.substring(0, indexOfStartingChars) + propValue
Copy link

Choose a reason for hiding this comment

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

Use a StringBuilder to avoid multiple concatenations.

+ text.substring(indexOfClosingBrace + 1);
}
if (sysProp.equals("carbon.home") && propValue != null
Copy link

Choose a reason for hiding this comment

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

Use a constant for "carbon.home"

&& propValue.equals(".")) {
Copy link

Choose a reason for hiding this comment

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

Use StringUtils.equals to avoid the explicit null check of "propValue".

text = new File(".").getAbsolutePath() + File.separator + text;
Copy link

Choose a reason for hiding this comment

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

Use a StringBuilder to avoid multiple concatenations.

}
}
return text;
}
}
4 changes: 4 additions & 0 deletions ...bon.user.core/src/main/java/org/wso2/carbon/user/core/config/RealmConfigXMLProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.osgi.framework.BundleContext;
import org.wso2.carbon.CarbonConstants;
import org.wso2.carbon.CarbonException;
import org.wso2.carbon.base.CarbonBaseUtils;
import org.wso2.carbon.user.api.RealmConfiguration;
import org.wso2.carbon.user.core.UserCoreConstants;
import org.wso2.carbon.user.core.UserStoreException;
Expand Down Expand Up @@ -181,6 +182,9 @@ private static void addPropertyElements(OMFactory factory, OMElement parent, Str
Map.Entry<String, String> entry = ite.next();
String name = entry.getKey();
String value = entry.getValue();
if (value != null) {
Copy link

Choose a reason for hiding this comment

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

Abstract the null check inside the replaceSystemProperty() method.
Refactor the other two usages of this method accordingly.

value = CarbonBaseUtils.replaceSystemProperty(value);
}
OMElement propElem = factory.createOMElement(new QName(
UserCoreConstants.RealmConfig.LOCAL_NAME_PROPERTY));
OMAttribute propAttr = factory.createOMAttribute(
Expand Down
3 changes: 3 additions & 0 deletions .../org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/util/DatabaseUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.tomcat.jdbc.pool.PoolProperties;
import org.wso2.carbon.base.CarbonBaseUtils;
import org.wso2.carbon.user.api.RealmConfiguration;
import org.wso2.carbon.user.core.UserStoreException;
import org.wso2.carbon.user.core.jdbc.JDBCRealmConstants;
Expand Down Expand Up @@ -95,6 +96,7 @@ public static DataSource createUserStoreDataSource(RealmConfiguration realmConfi

String dataSourceName = realmConfig.getUserStoreProperty(JDBCRealmConstants.DATASOURCE);
if (dataSourceName != null) {
dataSourceName = CarbonBaseUtils.replaceSystemProperty(dataSourceName);
return lookupDataSource(dataSourceName);
}

Expand Down Expand Up @@ -330,6 +332,7 @@ private static DataSource createRealmDataSource(RealmConfiguration realmConfig)

String dataSourceName = realmConfig.getRealmProperty(JDBCRealmConstants.DATASOURCE);
if (dataSourceName != null) {
dataSourceName = CarbonBaseUtils.replaceSystemProperty(dataSourceName);
return lookupDataSource(dataSourceName);
}

Expand Down