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

Embedded Tomcat support for custom webapp resources #701

Merged
merged 7 commits into from
Feb 2, 2024

Conversation

labkey-tchad
Copy link
Member

Rationale

A more generic solution for adding Resources to the webapp.
labkey.xml could have arbitrary <Resource> elements. This adds similar capability to application.properties.

This JMS resource from labkey.xml:

<Resource name="jms/ConnectionFactory" auth="Container"
        type="org.apache.activemq.ActiveMQConnectionFactory"
        factory="org.apache.activemq.jndi.JNDIReferenceFactory"
        description="JMS Connection Factory"
        brokerURL="vm://localhost?broker.persistent=false&amp;broker.useJmx=false"
        brokerName="LocalActiveMQBroker"/>

Can be represented like this in application.properties:

context.resources.jms.name=jms/ConnectionFactory
context.resources.jms.type=org.apache.activemq.ActiveMQConnectionFactory
context.resources.jms.factory=org.apache.activemq.jndi.JNDIReferenceFactory
context.resources.jms.description=JMS Connection Factory
context.resources.jms.brokerURL=vm://localhost?broker.persistent=false&broker.useJmx=false
context.resources.jms.brokerName=LocalActiveMQBroker

The jms in the property names is required to link related resource properties together but its exact text is unimportant.

Related Pull Requests

Changes

  • Add support for custom webapp resources

@labkey-tchad labkey-tchad changed the title Add support for custom webapp resources Embedded Tomcat support for custom webapp resources Jan 31, 2024
@labkey-jeckels
Copy link
Contributor

This looks great. I think it would let us rip out the jms. and ldap. examples from application.properties and the corresponding Java code in LabKeyServer, correct? The only downside is that it doesn't let us set default values for some of the properties, but we didn't have that in labkey.xml so that's not currently the expectation.

@labkey-jeckels
Copy link
Contributor

This also gives me an idea of how we can make the DataSource and extra webapp config a little cleaner...

Copy link
Contributor

@labkey-jeckels labkey-jeckels left a comment

Choose a reason for hiding this comment

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

See my one recommendation. After we merge the Tomcat 10 branch I'll rip out my more hard-coded handling for JMS and LDAP config.

resourceMap.putAll(entry.getValue());
if (!resourceMap.containsKey("name"))
{
logger.error("Resource configuration error: Ignoring unnamed resource 'context.resources.%s'".formatted(entry.getKey()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Some other config errors are fatal on startup, throwing an exception. I think that's probably the right direction to lean unless there's a good reason to be tolerant of an invalid config.

@labkey-tchad labkey-tchad merged commit 4ada1b6 into develop Feb 2, 2024
1 of 4 checks passed
@labkey-tchad labkey-tchad deleted the fb_embedded_extraContextResources branch February 2, 2024 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants