Skip to content

Commit

Permalink
Merge pull request #5142 from thehpi/issue-5095
Browse files Browse the repository at this point in the history 
FISH-1014 Variables (ENV=...) in @DatasourceDefinition not applied to 'className' #5095
  • Loading branch information
Alan authored Mar 11, 2021
2 parents 49bdd3b + db68863 commit f294b61
Show file tree
Hide file tree
Showing 4 changed files with 194 additions and 140 deletions.
7 changes: 6 additions & 1 deletion appserver/jdbc/jdbc-runtime/pom.xml
View file
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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;
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
Original file line number Diff line number Diff line change
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;
}
}


}
Loading

0 comments on commit f294b61

Please sign in to comment.