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

FISH-1014 Variables (ENV=...) in @DatasourceDefinition not applied to 'className' #5095 #5142

Merged
merged 3 commits into from Mar 11, 2021
Merged

FISH-1014 Variables (ENV=...) in @DatasourceDefinition not applied to 'className' #5095 #5142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
53ca19d
fixes #5095
Feb 26, 2021
e7b4035
issue-5095: added unit tests
Feb 26, 2021
db68863
issue-5095: Removed superfluous 'expand' method. Use mockito in unitt…
Mar 7, 2021
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
7 changes: 6 additions & 1 deletion appserver/jdbc/jdbc-runtime/pom.xml
View file
Open in desktop
Expand Up @@ -105,6 +105,11 @@
<artifactId>jakarta.el</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>

</dependencies>
</project>
38 changes: 19 additions & 19 deletions ...org/glassfish/jdbcruntime/deployment/annotation/handlers/DataSourceDefinitionHandler.java
View file
Open in desktop
Expand Up @@ -48,6 +48,7 @@
import org.glassfish.apf.AnnotationInfo;
import org.glassfish.apf.AnnotationProcessorException;
import org.glassfish.apf.HandlerProcessingResult;
import org.glassfish.config.support.TranslatedConfigView;
Copy link
Contributor

Choose a reason for hiding this comment

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

You could import static TranslatedConfigView.expandValue so you only use expandValue() so it could be more concise :)

import org.glassfish.deployment.common.JavaEEResourceType;
import org.glassfish.deployment.common.RootDeploymentDescriptor;
import org.jvnet.hk2.annotations.Service;
Expand Down Expand Up @@ -236,12 +237,12 @@ private void merge(Set<ResourceDescriptor> dsdDescs, DataSourceDefinition defn)
if (desc.getName().equals(defn.name())) {

if (desc.getClassName() == null) {
desc.setClassName(defn.className());
desc.setClassName(TranslatedConfigView.expandValue(defn.className()));
}

if (desc.getDescription() == null) {
if (defn.description() != null && !defn.description().equals("")) {
desc.setDescription(defn.description());
desc.setDescription(TranslatedConfigView.expandValue(defn.description()));
}
}

Expand All @@ -255,7 +256,7 @@ private void merge(Set<ResourceDescriptor> dsdDescs, DataSourceDefinition defn)
if (!desc.isServerNameSet() && desc.getUrl() == null) {
//localhost is the default value (even in the descriptor)
if (defn.serverName() != null && !defn.serverName().equals("localhost")) {
desc.setServerName(defn.serverName());
desc.setServerName(TranslatedConfigView.expandValue(defn.serverName()));
}
}

Expand All @@ -269,7 +270,7 @@ private void merge(Set<ResourceDescriptor> dsdDescs, DataSourceDefinition defn)
//try only when URL is not set
if (desc.getDatabaseName() == null && desc.getUrl() == null) {
if (defn.databaseName() != null && !defn.databaseName().equals("")) {
desc.setDatabaseName(defn.databaseName());
desc.setDatabaseName(TranslatedConfigView.expandValue(defn.databaseName()));
}
}

Expand All @@ -278,20 +279,20 @@ private void merge(Set<ResourceDescriptor> dsdDescs, DataSourceDefinition defn)
!(desc.getPortNumber() != -1 && desc.getServerName() != null &&
(desc.getDatabaseName() != null))) {
if (defn.url() != null && !defn.url().equals("")) {
desc.setUrl(defn.url());
desc.setUrl(TranslatedConfigView.expandValue(defn.url()));
}

}

if (desc.getUser() == null) {
if (defn.user() != null && !defn.user().equals("")) {
desc.setUser(defn.user());
desc.setUser(TranslatedConfigView.expandValue(defn.user()));
}
}

if (desc.getPassword() == null) {
if (defn.password() != null /*ALLOW EMPTY PASSWORDS && !defn.password().equals("")*/) {
desc.setPassword(defn.password());
desc.setPassword(TranslatedConfigView.expandValue(defn.password()));
}
}

Expand Down Expand Up @@ -357,7 +358,7 @@ private void merge(Set<ResourceDescriptor> dsdDescs, DataSourceDefinition defn)
String value = property.substring(index + 1);
//add to properties only when not already present
if (properties.get(name) == null) {
properties.put(name, value);
properties.put(name, TranslatedConfigView.expandValue(value));
}
}
}
Expand All @@ -374,15 +375,15 @@ DataSourceDefinitionDescriptor createDescriptor(DataSourceDefinition defn) {
DataSourceDefinitionDescriptor desc = new DataSourceDefinitionDescriptor();
desc.setMetadataSource(MetadataSource.ANNOTATION);

desc.setName(defn.name());
desc.setClassName(defn.className());
desc.setName(TranslatedConfigView.expandValue(defn.name()));
desc.setClassName(TranslatedConfigView.expandValue(defn.className()));

if (defn.description() != null && !defn.description().equals("")) {
desc.setDescription(defn.description());
desc.setDescription(TranslatedConfigView.expandValue(defn.description()));
}

if (defn.serverName() != null && !defn.serverName().equals("localhost")) {
desc.setServerName(defn.serverName());
desc.setServerName(TranslatedConfigView.expandValue(defn.serverName()));
}

if (defn.portNumber() != -1) {
Expand All @@ -391,28 +392,28 @@ DataSourceDefinitionDescriptor createDescriptor(DataSourceDefinition defn) {


if (defn.databaseName() != null && !defn.databaseName().equals("")) {
desc.setDatabaseName(defn.databaseName());
desc.setDatabaseName(TranslatedConfigView.expandValue(defn.databaseName()));
}

if (desc.getPortNumber() != -1 && desc.getDatabaseName() != null && desc.getServerName() != null) {
//standard properties are set, ignore URL
} else {
if (defn.url() != null && !defn.url().equals("")) {
desc.setUrl(defn.url());
desc.setUrl(TranslatedConfigView.expandValue(defn.url()));
desc.setServerName(null); //To prevent serverName overriding the URL, always use the URL if the standard properties are not set
}
}

if (defn.user() != null && !defn.user().equals("")) {
desc.setUser(defn.user());
desc.setUser(TranslatedConfigView.expandValue(defn.user()));
}

if (defn.password() != null /*ALLOW EMPTY PASSWORDS && !defn.password().equals("")*/) {
desc.setPassword(defn.password());
desc.setPassword(TranslatedConfigView.expandValue(defn.password()));
}

if (defn.isolationLevel() != -1) {
desc.setIsolationLevel(String.valueOf(defn.isolationLevel()));
desc.setIsolationLevel(TranslatedConfigView.expandValue(String.valueOf(defn.isolationLevel())));
}

if (defn.transactional()) {
Expand Down Expand Up @@ -443,7 +444,6 @@ DataSourceDefinitionDescriptor createDescriptor(DataSourceDefinition defn) {
desc.setLoginTimeout(String.valueOf(defn.loginTimeout()));
}


if (defn.properties() != null) {
Properties properties = desc.getProperties();

Expand All @@ -455,7 +455,7 @@ DataSourceDefinitionDescriptor createDescriptor(DataSourceDefinition defn) {
if (index > -1 && index != 0 && index < property.length() - 1) {
String name = property.substring(0, index);
String value = property.substring(index + 1);
properties.put(name, value);
properties.put(name, TranslatedConfigView.expandValue(value));
}
}
}
Expand Down
213 changes: 93 additions & 120 deletions ...va/org/glassfish/jdbcruntime/deployment/annotation/handlers/DataSourceDefinitionTest.java
View file
Open in desktop
Expand Up @@ -40,144 +40,117 @@
package org.glassfish.jdbcruntime.deployment.annotation.handlers;

import com.sun.enterprise.deployment.DataSourceDefinitionDescriptor;
import java.lang.annotation.Annotation;
import javax.annotation.sql.DataSourceDefinition;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;

import javax.annotation.sql.DataSourceDefinition;
import java.util.HashMap;
import java.util.Map;

import static org.mockito.Mockito.when;

/**
* Test for DataSourceDefinition processing in DataSourceDefinitionHandler
* @author jonathan coustick
*/
@RunWith(MockitoJUnitRunner.class)
public class DataSourceDefinitionTest {


@Mock
private DataSourceDefinition dataSourceDefinition;

@Before
public void before() {
when(dataSourceDefinition.portNumber()).thenReturn(-1);
when(dataSourceDefinition.isolationLevel()).thenReturn(-1);
when(dataSourceDefinition.transactional()).thenReturn(false);
when(dataSourceDefinition.initialPoolSize()).thenReturn(-1);
when(dataSourceDefinition.maxPoolSize()).thenReturn(-1);
when(dataSourceDefinition.minPoolSize()).thenReturn(-1);
when(dataSourceDefinition.maxIdleTime()).thenReturn(-1);
when(dataSourceDefinition.maxStatements()).thenReturn(-1);
when(dataSourceDefinition.loginTimeout()).thenReturn(-1);
}

@Test
public void test_environment_variable_expansion_works() throws Exception {
String url = "url";
String serverName = "server name";
String className = "class name";
String name = "name";
String description = "description";
String user = "user";
String password = "password";
String databaseName = "database name";
String property1 = "property 1";
String property2 = "property 2";

Map<String,String> env = new HashMap<>();
env.put("DB_URL", url);
env.put("DB_SERVER_NAME", serverName);
env.put("DB_CLASS_NAME", className);
env.put("DB_NAME", name);
env.put("DB_DESCRIPTION", description);
env.put("DB_USER", user);
env.put("DB_PASSWORD", password);
env.put("DB_DATABASE_NAME", databaseName);
env.put("DB_PROPERTY1", property1);
env.put("DB_PROPERTY2", property2);
EnvironmentUtil.setEnv(env);
DataSourceDefinitionHandler handler = new DataSourceDefinitionHandler();

when(dataSourceDefinition.url()).thenReturn("${ENV=DB_URL}");
when(dataSourceDefinition.serverName()).thenReturn("${ENV=DB_SERVER_NAME}");
when(dataSourceDefinition.className()).thenReturn("${ENV=DB_CLASS_NAME}");
when(dataSourceDefinition.name()).thenReturn("${ENV=DB_NAME}");
when(dataSourceDefinition.description()).thenReturn("${ENV=DB_DESCRIPTION}");
when(dataSourceDefinition.user()).thenReturn("${ENV=DB_USER}");
when(dataSourceDefinition.password()).thenReturn("${ENV=DB_PASSWORD}");
when(dataSourceDefinition.databaseName()).thenReturn("${ENV=DB_DATABASE_NAME}");
when(dataSourceDefinition.properties()).thenReturn(new String[]{"property1=${ENV=DB_PROPERTY1}", "property2=${ENV=DB_PROPERTY2}"});

DataSourceDefinitionDescriptor descriptor = handler.createDescriptor(dataSourceDefinition);

Assert.assertEquals(url,descriptor.getUrl());
Assert.assertNull(descriptor.getServerName()); // because url is set
Assert.assertEquals(className,descriptor.getClassName());
Assert.assertEquals(name,descriptor.getName());
Assert.assertEquals(description,descriptor.getDescription());
Assert.assertEquals(user,descriptor.getUser());
Assert.assertEquals(password,descriptor.getPassword());
Assert.assertEquals(databaseName,descriptor.getDatabaseName());
Assert.assertEquals(property1,descriptor.getProperty("property1"));
Assert.assertEquals(property2,descriptor.getProperty("property2"));
}

/**
* Test to ensure that if the URL has been set, it will override the default serverName of localhost
* and cause that to be set to null.
*/
@Test
public void testServerAndURL() {
DataSourceDefinitionHandler handler = new DataSourceDefinitionHandler();


String url = "http://database:5432/demo";
String serverName = "localhost";

//Check url overrides serverName and sets it to null
DataSourceDefinition definition = new DataSourceDefinitionImpl() {
@Override
public String url() {
return "http://database:5432/demo";
}

@Override
public String serverName() {
return "localhost";
}

};
DataSourceDefinitionDescriptor descriptor = handler.createDescriptor(definition);
when(dataSourceDefinition.url()).thenReturn(url);
when(dataSourceDefinition.serverName()).thenReturn(serverName);

DataSourceDefinitionDescriptor descriptor = handler.createDescriptor(dataSourceDefinition);

Assert.assertNull(descriptor.getServerName());



//Check if url is not set then serverName is left as-is
definition = new DataSourceDefinitionImpl() {
@Override
public String url() {
return null;
}

@Override
public String serverName() {
return "localhost";
}
};
descriptor = handler.createDescriptor(definition);
when(dataSourceDefinition.url()).thenReturn(null);

descriptor = handler.createDescriptor(dataSourceDefinition );

Assert.assertEquals("localhost", descriptor.getServerName());
}

abstract class DataSourceDefinitionImpl implements DataSourceDefinition {
@Override
public String name() {
return null;
}

@Override
public String className() {
return null;
}

@Override
public String description() {
return null;
}

@Override
public String user() {
return null;
}

@Override
public String password() {
return null;
}

@Override
public String databaseName() {
return null;
}

@Override
public int portNumber() {
return -1;
}

@Override
public int isolationLevel() {
return -1;
}

@Override
public boolean transactional() {
return false;
}

@Override
public int initialPoolSize() {
return -1;
}

@Override
public int maxPoolSize() {
return -1;
}

@Override
public int minPoolSize() {
return -1;
}

@Override
public int maxIdleTime() {
return -1;
}

@Override
public int maxStatements() {
return -1;
}

@Override
public String[] properties() {
return null;
}

@Override
public int loginTimeout() {
return -1;
}

@Override
public Class<? extends Annotation> annotationType() {
return DataSourceDefinition.class;
}
}


}