Skip to content

Fix Config Overriding During Tests #665

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
binh-dam-ibigroup wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Fix Config Overriding During Tests #665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a43b52d
fix(DataManager): Delete config node if setting null value
binh-dam-ibigroup Aug 17, 2026
dc9600a
refactor(DataManager): Apply light refactoring
binh-dam-ibigroup Aug 17, 2026
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
38 changes: 21 additions & 17 deletions src/main/java/com/conveyal/datatools/manager/DataManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import com.conveyal.datatools.common.utils.CorsFilter;
import com.conveyal.datatools.common.utils.RequestSummary;
import com.conveyal.datatools.common.utils.Scheduler;
import com.conveyal.datatools.common.utils.aws.S3Utils;
import com.conveyal.datatools.common.utils.SparkUtils;
import com.conveyal.datatools.editor.controllers.EditorLockController;
import com.conveyal.datatools.editor.controllers.api.EditorControllerImpl;
import com.conveyal.datatools.editor.controllers.api.SnapshotController;
Expand Down Expand Up @@ -55,7 +55,6 @@

import static com.conveyal.datatools.common.utils.SparkUtils.logMessageAndHalt;
import static com.conveyal.datatools.common.utils.SparkUtils.logRequest;
import static com.conveyal.datatools.common.utils.SparkUtils.logResponse;
import static spark.Service.SPARK_DEFAULT_PORT;
import static spark.Spark.after;
import static spark.Spark.before;
Expand Down Expand Up @@ -130,7 +129,7 @@ static void initializeApplication(String[] args, boolean initScheduledTasks) thr

// Optionally set port for server. Otherwise, Spark defaults to 4567.
if (hasConfigProperty("application.port")) {
PORT = Integer.parseInt(getConfigPropertyAsText("application.port"));
PORT = getConfigProperty("application.port").asInt();
port(PORT);
}
useS3 = "true".equals(getConfigPropertyAsText("application.data.use_s3_storage"));
Expand Down Expand Up @@ -339,9 +338,7 @@ static void registerRoutes() throws IOException {
});

// add logger
after((request, response) -> {
logResponse(request, response);
});
after(SparkUtils::logResponse);
}

/**
Expand All @@ -363,10 +360,10 @@ private static String resourceToString (String resourceName) {
}

private static boolean hasConfigProperty(JsonNode config, String name) {
String parts[] = name.split("\\.");
String[] parts = name.split("\\.");
JsonNode node = config;
for (int i = 0; i < parts.length; i++) {
if(node == null) return false;
if (node == null) return false;
node = node.get(parts[i]);
}
return node != null;
Expand All @@ -393,10 +390,10 @@ public static JsonNode getConfigProperty(String name) {
}

private static JsonNode getConfigProperty(JsonNode config, String name) {
String parts[] = name.split("\\.");
String[] parts = name.split("\\.");
JsonNode node = config;
for(int i = 0; i < parts.length; i++) {
if(node == null) {
for (int i = 0; i < parts.length; i++) {
if (node == null) {
LOG.warn("Config property {} not found", name);
return null;
}
Expand Down Expand Up @@ -455,18 +452,25 @@ public static boolean isExtensionEnabled(String extensionName) {
* In a test environment allows for overriding a specific config value on the server config object.
*/
public static void overrideConfigProperty(String name, String value) {
String parts[] = name.split("\\.");
String[] parts = name.split("\\.");
ObjectNode node = (ObjectNode) serverConfig;

//Loop through the dot separated field names to obtain final node and override that node's value.
// Loop through the dot separated field names to obtain final node and override that node's value.
for (int i = 0; i < parts.length; i++) {
String part = parts[i];
if (i < parts.length - 1) {
if (!node.has(parts[i])) {
node.set(parts[i], JsonUtil.objectMapper.createObjectNode());
if (!node.has(part)) {
node.set(part, JsonUtil.objectMapper.createObjectNode());
}
node = (ObjectNode) node.get(parts[i]);
node = (ObjectNode) node.get(part);
} else {
node.put(parts[i], value);
// If a value is null, delete the corresponding node instead of put-ting the node value
// (After node.put(part, null), calling getConfigPropertyAsText will return "null".)
if (value == null) {
node.remove(part);
} else {
node.put(part, value);
}
}
}
}
Expand Down
8 changes: 2 additions & 6 deletions src/test/java/com/conveyal/datatools/manager/extensions/mtc/MtcFeedResourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,7 @@ static void tearDown() {
@AfterEach
void resetState() {
DataManager.overrideConfigProperty(CONFIG_MTC_CREDENTIALS, defaultMtcAwsCredentials);

String currentMtcRegion = getConfigPropertyAsText(CONFIG_MTC_REGION);
if (currentMtcRegion != null && !currentMtcRegion.equals(defaultMtcAwsRegion)) {
DataManager.overrideConfigProperty(CONFIG_MTC_REGION, defaultMtcAwsRegion);
}
DataManager.overrideConfigProperty(CONFIG_MTC_REGION, defaultMtcAwsRegion);
}

@Test
Expand Down Expand Up @@ -262,7 +258,7 @@ void shouldUpdateRtdCarrierProperty(String address) throws JsonProcessingExcepti
}

@ParameterizedTest
@ValueSource(booleans = { false, true })
@ValueSource(booleans = { true, false })
void canUseCorrectMtcCredentials(boolean overwriteDefaults) {
if (overwriteDefaults) {
DataManager.overrideConfigProperty(CONFIG_MTC_REGION, "us-west-2");
Expand Down
Loading