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.

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

Fix to mend debug logging to allow programmatic level setting. #438

Merged
merged 3 commits into from Dec 3, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion org/postgresql/PGProperty.java
Expand Up @@ -463,7 +463,7 @@ public void set(Properties properties, int value)
*/
public boolean isPresent(Properties properties)
{
return get(properties) != null;
return getSetString(properties) != null;
}

/**
Expand Down Expand Up @@ -492,4 +492,17 @@ public static PGProperty forName(String name)
}
return null;
}
/**
* Return the property if exists but avoiding the default. Allowing the caller
* to detect the lack of a property.
* @param properties properties bundle
* @return the value of a set property
*/
public String getSetString(Properties properties)
{
Object o = properties.get(_name);
if (o instanceof String)
Copy link
Member

Choose a reason for hiding this comment

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

seems like you should check for null before checking instanceof ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

shouldn't be necessary. instanceof only works on the reference. not the object.
in the situation the reference is null then the instanceof condition returns false.

return (String) o;
return null;
}
}
15 changes: 12 additions & 3 deletions org/postgresql/jdbc2/AbstractJdbc2Connection.java
Expand Up @@ -8,7 +8,6 @@
package org.postgresql.jdbc2;

import java.io.IOException;
import java.io.PrintWriter;
import java.sql.*;
import java.util.*;

Expand Down Expand Up @@ -122,12 +121,22 @@ protected AbstractJdbc2Connection(HostSpec[] hostSpecs, String user, String data
{
this.creatingURL = url;

// Read loglevel arg and set the loglevel based on this value;
// Read loglevel arg and set the loglevel based on this value
// otherwise use the programmatic or init logger level;
// In addition to setting the log level, enable output to
// standard out if no other printwriter is set

int logLevel = Driver.getLogLevel();

String connectionLogLevel = PGProperty.LOG_LEVEL.getSetString(info);
if (connectionLogLevel != null) {
try
{
logLevel = Integer.parseInt(connectionLogLevel);
} catch (NumberFormatException nfe)
{
// ignore
}
}
synchronized (AbstractJdbc2Connection.class) {
logger = new Logger(nextConnectionID++);
logger.setLogLevel(logLevel);
Expand Down
13 changes: 12 additions & 1 deletion org/postgresql/test/TestUtil.java
Expand Up @@ -58,6 +58,11 @@ public static String getURL(String server, int port)
sendBufferSize = "&sendBufferSize="+getSendBufferSize();
}

String ssl = "";
if (getSSL() != null ){
ssl = "&ssl="+getSSL();
}

return "jdbc:postgresql://"
+ server + ":"
+ port + "/"
Expand All @@ -66,7 +71,8 @@ public static String getURL(String server, int port)
+ protocolVersion
+ binaryTransfer
+ receiveBufferSize
+ sendBufferSize;
+ sendBufferSize
+ ssl;
}

/*
Expand Down Expand Up @@ -161,6 +167,11 @@ public static int getReceiveBufferSize()
return Integer.parseInt(System.getProperty("receiveBufferSize","-1"));
}

public static String getSSL()
{
return System.getProperty("ssl");
}

private static boolean initialized = false;
public static void initDriver() throws Exception
{
Expand Down
47 changes: 47 additions & 0 deletions org/postgresql/test/jdbc2/PGPropertyTest.java
Expand Up @@ -204,9 +204,13 @@ public void testIsPresentWithParseURLResult() throws Exception
givenProperties.setProperty("user", TestUtil.getUser());
givenProperties.setProperty("password", TestUtil.getPassword());

Properties sysProperties = System.getProperties();
sysProperties.remove("ssl");
System.setProperties(sysProperties);
Properties parsedProperties = Driver.parseURL(TestUtil.getURL(), givenProperties);
Assert.assertFalse("SSL property should not be present", PGProperty.SSL.isPresent(parsedProperties));

System.setProperty("ssl", "true");
givenProperties.setProperty("ssl", "true");
parsedProperties = Driver.parseURL(TestUtil.getURL(), givenProperties);
Assert.assertTrue("SSL property should be present", PGProperty.SSL.isPresent(parsedProperties));
Expand All @@ -218,4 +222,47 @@ public void testIsPresentWithParseURLResult() throws Exception
parsedProperties = Driver.parseURL(TestUtil.getURL() + "&ssl=true" , null);
Assert.assertTrue("SSL property should be present", PGProperty.SSL.isPresent(parsedProperties));
}

/**
* Check whether the isPresent method really works.
*/
public void testPresenceCheck()
{
Properties empty = new Properties();
Object value = PGProperty.LOG_LEVEL.get(empty);
assertNotNull(value);
Assert.assertFalse(PGProperty.LOG_LEVEL.isPresent(empty));
}

public void testNullValue()
{
Properties empty = new Properties();
assertNull(PGProperty.LOG_LEVEL.getSetString(empty));
Properties withLogging = new Properties();
withLogging.setProperty(PGProperty.LOG_LEVEL.getName(), "2");
assertNotNull(PGProperty.LOG_LEVEL.getSetString(withLogging));
Copy link
Member

Choose a reason for hiding this comment

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

Could you please move that to separate test method?

}

public void setUp()
{
bootSSLPropertyValue = System.getProperty("ssl");
}

public void tearDown()
{
if (bootSSLPropertyValue == null) {
System.getProperties().remove("ssl");
}
else
{
System.setProperty("ssl", bootSSLPropertyValue);
}
}

/**
* Some tests modify the "ssl" system property. To not disturb
* other test cases in the suite store the value of the property
* and restore it.
*/
private String bootSSLPropertyValue;
}