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

[thehpi]

Description

In a @DatasourceDefinition the 'className' property cannot be configured using "${ENV:ENVIRONMENT_VARIABLE_NAME}"

If I use this in the web.xml however (currently web.xml.old) then it does work!

Reproduce

See this github repo for the issue description and to reproduce it.

[AlanRoth]

Hi @thehpi,

Following your reproducer steps, the build fails with PSQLException: The connection attempt failed.. Any advice on what I may be doing wrong?

Thank you,
Alan

[thehpi]

Hi, Yes this is exactly the problem. It cannot get a connection because the className cannot be determined from the DB_DRIVER environment variable. If you replace the env var with the correct class then it works. If this is not what you mean please send the full stacktrace. Gr Hans

…

Op 15 jan. 2021 om 16:49 heeft Alan ***@***.***> het volgende geschreven:  Hi @thehpi, Following your reproducer steps, the build fails with PSQLException: The connection attempt failed.. Any advice on what I may be doing wrong? Thank you, Alan — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[thehpi]

Ow I now see what you mean, the build is failing because of a test which assumes the db is running.

I removed the test, now it builds so you can reproduce. Apologies.

[AlanRoth]

Hi @thehpi,

I have been able to reproduce the issue with the change, I created an internal ticket FISH-1014 to keep track of this. Are you able to submit a PR to fix this? Due to the influx of community contributions our dev team may take a while to get around to fixing this.

Thank you,
Alan

[AlanRoth]

Hi @thehpi,

It seems like #5024 may be related to your issue, could you confirm that this is also related to your issue? I do not want to prematurely close this issue without your input if you determine they are different issues.

Thank you,
Alan

[thehpi]

It is related but not the same problem as in my case it works for all properties except the className. Also in the example in the other ticket the className is not using the placeholder. So I guess this is a different problem

…

Op 19 jan. 2021 om 12:58 heeft Alan ***@***.***> het volgende geschreven:  Hi @thehpi, It seems like #5024 may be related to your issue, could you confirm that this is also related to your issue? I do not want to prematurely close this issue without your input if you determine they are different issues. Thank you, Alan — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[poikilotherm]

Suspecting that

Payara/appserver/jdbc/jdbc-runtime/src/main/java/org/glassfish/jdbc/deployer/JdbcConnectionPoolDeployer.java

Lines 303 to 324 in a76cdb4

	if (adminPool.getResType() != null) {
	if (ConnectorConstants.JAVA_SQL_DRIVER.equals(adminPool.getResType())) {
	propList.add(new ConnectorConfigProperty("ClassName",
	adminPool.getDriverClassname() == null ? "" : adminPool.getDriverClassname(),
	"The driver class name", "java.lang.String"));
	} else {
	propList.add(new ConnectorConfigProperty("ClassName",
	adminPool.getDatasourceClassname() == null ? "" : adminPool.getDatasourceClassname(),
	"The datasource class name", "java.lang.String"));
	}
	} else {
	//When resType is null, one of these classnames would be specified
	if (adminPool.getDriverClassname() != null) {
	propList.add(new ConnectorConfigProperty("ClassName",
	adminPool.getDriverClassname() == null ? "" : adminPool.getDriverClassname(),
	"The driver class name", "java.lang.String"));
	} else if (adminPool.getDatasourceClassname() != null) {
	propList.add(new ConnectorConfigProperty("ClassName",
	adminPool.getDatasourceClassname() == null ? "" : adminPool.getDatasourceClassname(),
	"The datasource class name", "java.lang.String"));
	}
	}

does the class name magic, it looks like you cannot use variable substitution for the very same reasons as outlined in #5024.

[thehpi]

@poikilotherm, I read #5024 and used TranslatedConfigView to expand the className.

if (adminPool.getResType() != null) {
    if (ConnectorConstants.JAVA_SQL_DRIVER.equals(adminPool.getResType())) {
        propList.add(new ConnectorConfigProperty("ClassName",
                adminPool.getDriverClassname() == null ? "" : TranslatedConfigView.expandValue(adminPool.getDriverClassname()),
                "The driver class name", "java.lang.String"));
    } else {
        propList.add(new ConnectorConfigProperty("ClassName",
                adminPool.getDatasourceClassname() == null ? "" : TranslatedConfigView.expandValue(adminPool.getDatasourceClassname()),
                "The datasource class name", "java.lang.String"));
    }
} else {
    //When resType is null, one of these classnames would be specified
    if (adminPool.getDriverClassname() != null) {
        propList.add(new ConnectorConfigProperty("ClassName",
                adminPool.getDriverClassname() == null ? "" : TranslatedConfigView.expandValue(adminPool.getDriverClassname()),
                "The driver class name", "java.lang.String"));
    } else if (adminPool.getDatasourceClassname() != null) {
        propList.add(new ConnectorConfigProperty("ClassName",
                adminPool.getDatasourceClassname() == null ? "" : TranslatedConfigView.expandValue(adminPool.getDatasourceClassname()),
                "The datasource class name", "java.lang.String"));
    }
}

But I'm getting a setUrl access error.

I think the problem is that adminPool.getDatasourceClassname() is used in many places where the expansion is not performed and doing it at this point causes inconsistencies.

So it will probably better to do the expansion in DataSourceDefinitionHandler.