pinal-shah commented on code in PR #476:
URL: https://github.com/apache/atlas/pull/476#discussion_r2575689209
##########
intg/pom.xml:
##########
@@ -115,6 +115,12 @@
</exclusions>
</dependency>
+ <dependency>
Review Comment:
avoid using extra dependency if not needed
##########
intg/src/test/java/org/apache/atlas/ApplicationPropertiesTest.java:
##########
@@ -132,12 +140,12 @@ public void testGetFileAsInputStream() throws Exception {
@Test
public void verifySetDefault() throws AtlasException {
- Configuration props =
ApplicationProperties.get("test.properties");
+ Configuration props = ApplicationProperties.get("test.properties");
Review Comment:
retain original formatting of this code
##########
intg/src/test/java/org/apache/atlas/ApplicationPropertiesTest.java:
##########
@@ -148,13 +156,73 @@ public void verifySetDefault() throws AtlasException {
@Test
public void verifyGetLatesttString() throws AtlasException {
- String key = "atlas.metadata.namespace";
- String oldVal = "nm-sp-1";
- String newVal = "nm-sp-2";
+ String key = "atlas.metadata.namespace";
Review Comment:
retain original formatting
##########
intg/src/test/java/org/apache/atlas/ApplicationPropertiesTest.java:
##########
@@ -19,20 +19,28 @@
import org.apache.atlas.utils.AtlasConfigurationUtil;
import org.apache.commons.configuration.Configuration;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
import org.testng.annotations.Test;
+import java.io.IOException;
import java.io.InputStream;
import java.util.AbstractMap;
import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotEquals;
import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertSame;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.expectThrows;
import static org.testng.Assert.fail;
/**
* Unit test for {@link ApplicationProperties}
*
*/
+
Review Comment:
nit
##########
intg/src/test/java/org/apache/atlas/ApplicationPropertiesTest.java:
##########
@@ -148,13 +156,73 @@ public void verifySetDefault() throws AtlasException {
@Test
public void verifyGetLatesttString() throws AtlasException {
- String key = "atlas.metadata.namespace";
- String oldVal = "nm-sp-1";
- String newVal = "nm-sp-2";
+ String key = "atlas.metadata.namespace";
+ String oldVal = "nm-sp-1";
+ String newVal = "nm-sp-2";
Configuration atlasConf = ApplicationProperties.get("test.properties");
assertEquals(atlasConf.getString(key), oldVal);
assertEquals(AtlasConfigurationUtil.getRecentString(atlasConf, key,
oldVal), newVal);
assertEquals(AtlasConfigurationUtil.getRecentString(atlasConf,
"garbage", oldVal), oldVal);
}
+
+ @Test
+ public void verifyCustomisedPath() throws AtlasException, IOException {
+ Configuration atlasConf =
ApplicationProperties.getConf("src/test/resources/test.properties");
+ InputStream inStr = null;
+
+ try {
+ inStr = ApplicationProperties.getFileAsInputStream(atlasConf,
"jaas.properties.file", null);
+ assertNotNull(inStr);
+ } finally {
+ if (inStr != null) {
+ inStr.close();
+ }
+ }
+ }
+
+ @Test
+ public void verifyPropertyValues() throws AtlasException {
+ Configuration props =
ApplicationProperties.getConf("src/test/resources/test.properties");
+ ApplicationProperties aProps = (ApplicationProperties) props;
+
+ String defaultValue = "atlas";
+ String someKey = "atlas.service";
+
+ assertFalse(aProps.getString(someKey).equals(defaultValue));
Review Comment:
can you add true value as well
##########
intg/src/test/java/org/apache/atlas/ApplicationPropertiesTest.java:
##########
@@ -148,13 +156,73 @@ public void verifySetDefault() throws AtlasException {
@Test
public void verifyGetLatesttString() throws AtlasException {
- String key = "atlas.metadata.namespace";
- String oldVal = "nm-sp-1";
- String newVal = "nm-sp-2";
+ String key = "atlas.metadata.namespace";
+ String oldVal = "nm-sp-1";
+ String newVal = "nm-sp-2";
Configuration atlasConf = ApplicationProperties.get("test.properties");
assertEquals(atlasConf.getString(key), oldVal);
assertEquals(AtlasConfigurationUtil.getRecentString(atlasConf, key,
oldVal), newVal);
assertEquals(AtlasConfigurationUtil.getRecentString(atlasConf,
"garbage", oldVal), oldVal);
}
+
+ @Test
+ public void verifyCustomisedPath() throws AtlasException, IOException {
+ Configuration atlasConf =
ApplicationProperties.getConf("src/test/resources/test.properties");
+ InputStream inStr = null;
+
+ try {
+ inStr = ApplicationProperties.getFileAsInputStream(atlasConf,
"jaas.properties.file", null);
Review Comment:
please review if this is required
##########
intg/src/test/java/org/apache/atlas/ApplicationPropertiesTest.java:
##########
@@ -148,13 +156,73 @@ public void verifySetDefault() throws AtlasException {
@Test
public void verifyGetLatesttString() throws AtlasException {
- String key = "atlas.metadata.namespace";
- String oldVal = "nm-sp-1";
- String newVal = "nm-sp-2";
+ String key = "atlas.metadata.namespace";
+ String oldVal = "nm-sp-1";
+ String newVal = "nm-sp-2";
Configuration atlasConf = ApplicationProperties.get("test.properties");
assertEquals(atlasConf.getString(key), oldVal);
assertEquals(AtlasConfigurationUtil.getRecentString(atlasConf, key,
oldVal), newVal);
assertEquals(AtlasConfigurationUtil.getRecentString(atlasConf,
"garbage", oldVal), oldVal);
}
+
+ @Test
+ public void verifyCustomisedPath() throws AtlasException, IOException {
+ Configuration atlasConf =
ApplicationProperties.getConf("src/test/resources/test.properties");
+ InputStream inStr = null;
+
+ try {
+ inStr = ApplicationProperties.getFileAsInputStream(atlasConf,
"jaas.properties.file", null);
+ assertNotNull(inStr);
+ } finally {
+ if (inStr != null) {
+ inStr.close();
+ }
+ }
+ }
+
+ @Test
+ public void verifyPropertyValues() throws AtlasException {
+ Configuration props =
ApplicationProperties.getConf("src/test/resources/test.properties");
+ ApplicationProperties aProps = (ApplicationProperties) props;
+
+ String defaultValue = "atlas";
+ String someKey = "atlas.service";
+
+ assertFalse(aProps.getString(someKey).equals(defaultValue));
+ }
+
+ @Test
+ public void verifyCustomisedPathFailureExpected() {
+ try (MockedStatic<ApplicationProperties> mocked =
Mockito.mockStatic(ApplicationProperties.class)) {
Review Comment:
not required, if you find issues, you can use
ApplicationProperties.forceReload()
##########
intg/src/main/java/org/apache/atlas/ApplicationProperties.java:
##########
@@ -124,36 +124,68 @@ public static Configuration set(Configuration
configuration) throws AtlasExcepti
return instance;
}
- public static Configuration get(String fileName) throws AtlasException {
- String confLocation =
System.getProperty(ATLAS_CONFIGURATION_DIRECTORY_PROPERTY);
+ public static Configuration getConf(String propertyFileName) throws
AtlasException {
+ if (instance == null) {
+ synchronized (ApplicationProperties.class) {
+ if (instance == null) {
+ set(get(propertyFileName));
+ }
+ }
+ }
+ return instance;
+ }
- try {
- URL url;
+ public static Configuration getConf(Configuration clientConf) throws
AtlasException {
+ if (instance == null) {
+ synchronized (ApplicationProperties.class) {
+ if (instance == null) {
+ set(clientConf);
+ }
+ }
+ }
+ return instance;
+ }
- if (confLocation == null) {
- LOG.info("Looking for {} in classpath", fileName);
+ public static Configuration get(String fileName) throws AtlasException {
+ try {
+ URL url = null;
+
+ if (StringUtils.isNotBlank(fileName)) {
+ File file = new File(fileName);
+ boolean hasPath = file.getParent() != null;
+ if (hasPath) {
Review Comment:
Why is this check needed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
