Author: kwall Date: Fri Apr 24 13:35:49 2015 New Revision: 1675856 URL: http://svn.apache.org/r1675856 Log: QPID-6506, QPID-6508: [Java Client] PropertiesFileInitialContextFactory no longer swallows exceptions, pollutes the system properties, nor modifies the environment.
IOExceptions and URISyntaxExceptions that were previously swallowed are now chained to a NamingException. If the environment needs to be modified within the method a copy is created. System properties are no longer set. work done by Lorenz Quack <[email protected]> and Keith Wall <[email protected]> Modified: qpid/java/trunk/client/src/main/java/org/apache/qpid/jndi/PropertiesFileInitialContextFactory.java qpid/java/trunk/client/src/test/java/org/apache/qpid/jndi/PropertiesFileInitialContextFactoryTest.java qpid/java/trunk/pom.xml qpid/java/trunk/qpid-systests-parent/pom.xml Modified: qpid/java/trunk/client/src/main/java/org/apache/qpid/jndi/PropertiesFileInitialContextFactory.java URL: http://svn.apache.org/viewvc/qpid/java/trunk/client/src/main/java/org/apache/qpid/jndi/PropertiesFileInitialContextFactory.java?rev=1675856&r1=1675855&r2=1675856&view=diff ============================================================================== --- qpid/java/trunk/client/src/main/java/org/apache/qpid/jndi/PropertiesFileInitialContextFactory.java (original) +++ qpid/java/trunk/client/src/main/java/org/apache/qpid/jndi/PropertiesFileInitialContextFactory.java Fri Apr 24 13:35:49 2015 @@ -68,18 +68,21 @@ public class PropertiesFileInitialContex @SuppressWarnings({ "rawtypes", "unchecked" }) public Context getInitialContext(Hashtable environment) throws NamingException { - Map data = new ConcurrentHashMap(); - BufferedInputStream inputStream = null; + environment = (environment == null) ? new Hashtable() : environment; + String fileName = (environment.containsKey(Context.PROVIDER_URL)) ? (String)environment.get(Context.PROVIDER_URL) : System.getProperty(Context.PROVIDER_URL); - try + + if (fileName != null) { - if (fileName != null) + try (BufferedInputStream inputStream = new BufferedInputStream(new FileInputStream((fileName.contains("file:")) + ? new File(new URI(fileName)) + : new File(fileName)))) { - _logger.debug("Attempting to load " + fileName); + // make copy of the original environment to adhere to the Contexts interface + environment = new Hashtable(environment); + _logger.debug("Attempting to load '{}'", fileName); - inputStream = new BufferedInputStream(new FileInputStream((fileName.contains("file:")) - ? new File(new URI(fileName)) : new File(fileName))); Properties p = new Properties(); p.load(inputStream); @@ -92,39 +95,21 @@ public class PropertiesFileInitialContex String value = (String) me.getValue(); String expanded = Strings.expand(value, resolver); environment.put(key, expanded); - if (System.getProperty(key) == null) - { - System.setProperty(key, expanded); - } } } - else + catch (IOException | URISyntaxException e) { - _logger.debug("No Provider URL specified."); + NamingException ne = new NamingException("Unable to load property file specified in Provider_URL:" + fileName + "."); + ne.setRootCause(e); + throw ne; } } - catch (IOException ioe) - { - _logger.warn("Unable to load property file specified in Provider_URL:" + fileName +"\n" + - "Due to:" + ioe.getMessage()); - } - catch(URISyntaxException uoe) - { - _logger.warn("Unable to load property file specified in Provider_URL:" + fileName +"\n" + - "Due to:" + uoe.getMessage()); - } - finally + else { - try - { - if(inputStream != null) - { - inputStream.close(); - } - } - catch(Exception ignore){} + _logger.debug("{} was not specified in the context's environment or as a system property.", Context.PROVIDER_URL); } + Map data = new ConcurrentHashMap(); createConnectionFactories(data, environment); createDestinations(data, environment); Modified: qpid/java/trunk/client/src/test/java/org/apache/qpid/jndi/PropertiesFileInitialContextFactoryTest.java URL: http://svn.apache.org/viewvc/qpid/java/trunk/client/src/test/java/org/apache/qpid/jndi/PropertiesFileInitialContextFactoryTest.java?rev=1675856&r1=1675855&r2=1675856&view=diff ============================================================================== --- qpid/java/trunk/client/src/test/java/org/apache/qpid/jndi/PropertiesFileInitialContextFactoryTest.java (original) +++ qpid/java/trunk/client/src/test/java/org/apache/qpid/jndi/PropertiesFileInitialContextFactoryTest.java Fri Apr 24 13:35:49 2015 @@ -24,6 +24,7 @@ package org.apache.qpid.jndi; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.util.Hashtable; import java.util.Properties; import javax.jms.ConnectionFactory; @@ -134,4 +135,26 @@ public class PropertiesFileInitialContex f.delete(); } } + + public void testNonExistentFileHandling() throws Exception + { + Hashtable environment = new Hashtable(); + // Make sure the filename does not exist + File f = File.createTempFile(getTestName(), ".properties"); + f.delete(); + + environment.put(Context.PROVIDER_URL, f.getAbsolutePath()); + environment.put(InitialContext.INITIAL_CONTEXT_FACTORY, "org.apache.qpid.jndi.PropertiesFileInitialContextFactory"); + + try + { + new InitialContext(environment); + fail("Expected exception not thrown."); + } + catch (NamingException e) + { + assertTrue("No IOException chained", e.getCause() instanceof IOException); + } + + } } Modified: qpid/java/trunk/pom.xml URL: http://svn.apache.org/viewvc/qpid/java/trunk/pom.xml?rev=1675856&r1=1675855&r2=1675856&view=diff ============================================================================== --- qpid/java/trunk/pom.xml (original) +++ qpid/java/trunk/pom.xml Fri Apr 24 13:35:49 2015 @@ -214,7 +214,6 @@ <QPID_HOME>${qpid.home}</QPID_HOME> <QPID_WORK>${qpid.work}</QPID_WORK> <java.naming.factory.initial>org.apache.qpid.jndi.PropertiesFileInitialContextFactory</java.naming.factory.initial> - <java.naming.provider.url>test-profiles${file.separator}test-provider.properties</java.naming.provider.url> <broker.config>${qpid.home}${file.separator}etc${file.separator}config-systests.json</broker.config> <max_prefetch>1000</max_prefetch> <qpid.dest_syntax>BURL</qpid.dest_syntax> Modified: qpid/java/trunk/qpid-systests-parent/pom.xml URL: http://svn.apache.org/viewvc/qpid/java/trunk/qpid-systests-parent/pom.xml?rev=1675856&r1=1675855&r2=1675856&view=diff ============================================================================== --- qpid/java/trunk/qpid-systests-parent/pom.xml (original) +++ qpid/java/trunk/qpid-systests-parent/pom.xml Fri Apr 24 13:35:49 2015 @@ -84,6 +84,7 @@ integration-test phase below instead --> <skip>true</skip> <systemPropertyVariables> + <java.naming.provider.url>test-profiles${file.separator}test-provider.properties</java.naming.provider.url> <test.output.dir>${test.output.dir}</test.output.dir> <log4j.configuration.file>${basedir}${file.separator}target${file.separator}classes/log4j.xml</log4j.configuration.file> <!-- Let the tests themselves configure logging so that we can have a separate log file per test --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
