Skip to content

Read env vars for server name and env name #46

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

Open
rogin wants to merge 1 commit into
base: main
Choose a base branch
from
+44 −4
Open

Read env vars for server name and env name #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions server/src/com/mirth/connect/server/Mirth.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import java.sql.DriverManager;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.List;
import java.util.Timer;
import java.util.TimerTask;
Expand All @@ -29,10 +28,10 @@
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.math.NumberUtils;
import org.apache.logging.log4j.ThreadContext;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.ThreadContext;
import org.apache.logging.log4j.core.Appender;
import org.apache.logging.log4j.core.filter.Filterable;
import org.apache.velocity.runtime.RuntimeConstants;
Expand Down Expand Up @@ -345,9 +344,11 @@ public void startup() {
((org.apache.logging.log4j.core.Logger) velocityLogger).setLevel(Level.OFF);
}

configurationController.updateServerSettingsFromEnvironment();
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

This startup call can throw AbstractMethodError at runtime if an extension supplies a custom ConfigurationController compiled against an older version (missing updateServerSettingsFromEnvironment). Either ensure the base class provides a default implementation, or guard this call (e.g., catch AbstractMethodError) so third-party controllers don’t prevent server startup.

Suggested change
configurationController.updateServerSettingsFromEnvironment();
try {
configurationController.updateServerSettingsFromEnvironment();
} catch (AbstractMethodError e) {
logger.warn("ConfigurationController implementation does not support updateServerSettingsFromEnvironment; skipping environment-based server settings update for compatibility.", e);
}

Copilot uses AI. Check for mistakes.

eventController.dispatchEvent(new ServerEvent(configurationController.getServerId(), "Server startup"));

// Start web server before starting the engine in case there is a
// Start web server before starting the engine in case there is a
// problem starting the engine that causes it to hang
startWebServer();

Expand Down
5 changes: 5 additions & 0 deletions server/src/com/mirth/connect/server/controllers/ConfigurationController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,4 +360,9 @@ public Properties getPropertiesForGroup(String group) {
public abstract void setChannelTags(Set<ChannelTag> tags);

public abstract Set<ChannelTag> getChannelTags();

/**
* Update server settings based on environment variables.
*/
public abstract void updateServerSettingsFromEnvironment();
Comment on lines +366 to +367
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Adding a new abstract method here is a binary-breaking change for any extension-provided ConfigurationController. DefaultConfigurationController.create() can load a custom controller via ExtensionLoader, and if that implementation was compiled against an older version it won’t implement this method, leading to AbstractMethodError when the server calls it during startup. Prefer making this a concrete no-op method (overrideable) to preserve backward compatibility for extensions.

Suggested change
*/
public abstract void updateServerSettingsFromEnvironment();
* <p>
* Default implementation is a no-op to preserve backward compatibility for
* existing ConfigurationController implementations.
*/
public void updateServerSettingsFromEnvironment() {
// No-op by default.
}

Copilot uses AI. Check for mistakes.
}
36 changes: 35 additions & 1 deletion server/src/com/mirth/connect/server/controllers/DefaultConfigurationController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.Properties;
import java.util.Set;
import java.util.SortedMap;
Expand Down Expand Up @@ -75,7 +76,6 @@
import org.apache.ibatis.session.SqlSessionManager;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.bouncycastle.asn1.ASN1Sequence;
import org.bouncycastle.asn1.x500.X500Name;
import org.bouncycastle.asn1.x509.AuthorityKeyIdentifier;
import org.bouncycastle.asn1.x509.BasicConstraints;
Expand Down Expand Up @@ -1704,4 +1704,38 @@ public ConnectionTestResponse sendTestEmail(Properties properties) throws Except
return new ConnectionTestResponse(ConnectionTestResponse.Type.FAILURE, e.getMessage());
}
}

@Override
public void updateServerSettingsFromEnvironment() {
Optional<String> newServerName = getEnvironmentVariable("MC_SERVER_NAME");
Optional<String> newEnvName = getEnvironmentVariable("MC_ENV_NAME");

if (newServerName.isPresent() || newEnvName.isPresent()) {
try {
ServerSettings serverSettings = getServerSettings();

if (newServerName.isPresent()) {
serverSettings.setServerName(newServerName.get());
}
if (newEnvName.isPresent()) {
serverSettings.setEnvironmentName(newEnvName.get());
}

setServerSettings(serverSettings);
Comment on lines +1709 to +1724
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

This new env-var-driven startup behavior isn’t covered by automated tests. Since server/test already has DefaultConfigurationController tests, please add tests that verify: (1) when env vars are absent/blank, DB values aren’t overwritten; (2) when present, the correct properties are saved. To make this testable without relying on mutating System.getenv, consider extracting env access behind an overrideable method or injectable supplier.

Copilot uses AI. Check for mistakes.
} catch (ControllerException e) {
Comment on lines +1713 to +1725
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

updateServerSettingsFromEnvironment() currently calls getServerSettings() + setServerSettings(). setServerSettings() persists all core properties and saveProperty() may vacuum the configuration table each time, which can add a lot of DB writes/locks on every startup whenever either env var is set. Consider updating only the specific properties (environment.name and server.name) instead of calling setServerSettings(serverSettings).

Copilot uses AI. Check for mistakes.
logger.error("Failed to update server settings via environment variables", e);
}
}
}

/**
* Pull an environment variable. Values are trimmed, and only non-empty values are returned.
*
* @param envName the environment variable name
* @return the property's value
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Javadoc is a bit misleading here: this method reads environment variables, not properties. Consider updating @return to describe the environment variable’s value (or Optional empty) to avoid confusion.

Suggested change
* @return the property's value
* @return an {@link Optional} containing the trimmed environment variable value if present and non-blank;
* otherwise {@link Optional#empty()}

Copilot uses AI. Check for mistakes.
*/
private Optional<String> getEnvironmentVariable(String envName) {
String propValue = StringUtils.trimToEmpty(System.getenv(envName));
return StringUtils.isNotBlank(propValue) ? Optional.of(propValue) : Optional.empty();
}
}
Loading