Fix config file save logic in FileBasedMessageStore Previously, the `saveConfiguredMessage()` method was calling `configFile.load()` after updating the config values with addAll(), which reloaded the file from disk and discarded all in-memory changes. This resulted in config updates being lost while only the HTML file was saved. Fix this by removing the `configFile.load()` call ensuring that configuration changes are properly persisted to the config file along with the HTML content. Change-Id: I60dd739e6e74802437164ef218ef98eb0afdf0f7
diff --git a/BUILD b/BUILD index b517141..95ed97f 100644 --- a/BUILD +++ b/BUILD
@@ -1,5 +1,12 @@ -load("//tools/bzl:plugin.bzl", "gerrit_plugin") +load("@rules_java//java:defs.bzl", "java_library") load("//tools/bzl:js.bzl", "gerrit_js_bundle") +load("//tools/bzl:junit.bzl", "junit_tests") +load( + "//tools/bzl:plugin.bzl", + "PLUGIN_DEPS", + "PLUGIN_TEST_DEPS", + "gerrit_plugin", +) gerrit_plugin( name = "messageoftheday", @@ -20,3 +27,17 @@ srcs = glob(["gr-messageoftheday/*.js"]), entry_point = "gr-messageoftheday/plugin.js", ) + +junit_tests( + name = "messageoftheday_tests", + srcs = glob(["src/test/java/**/*.java"]), + tags = ["messageoftheday"], + deps = [":messageoftheday__plugin_test_deps"], +) + +java_library( + name = "messageoftheday__plugin_test_deps", + testonly = 1, + visibility = ["//visibility:public"], + exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [":messageoftheday__plugin"], +)
diff --git a/src/main/java/com/googlesource/gerrit/plugins/messageoftheday/FileBasedMessageStore.java b/src/main/java/com/googlesource/gerrit/plugins/messageoftheday/FileBasedMessageStore.java index 4c40d85..4a3e8dc 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/messageoftheday/FileBasedMessageStore.java +++ b/src/main/java/com/googlesource/gerrit/plugins/messageoftheday/FileBasedMessageStore.java
@@ -82,7 +82,7 @@ @Override public void saveConfiguredMessage(ConfiguredMessage message) throws MessageStoreException { - FileBasedConfig configFile = new FileBasedConfig(message.config(), cfgFile, FS.DETECTED); + FileBasedConfig configFile = new FileBasedConfig(cfgFile, FS.DETECTED); addAll(configFile, message.config()); @@ -96,9 +96,8 @@ } try { - configFile.load(); configFile.save(); - } catch (IOException | ConfigInvalidException e) { + } catch (IOException e) { throw new MessageStoreException("Failed to save config", e); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/messageoftheday/FileBasedMessageStoreTest.java b/src/test/java/com/googlesource/gerrit/plugins/messageoftheday/FileBasedMessageStoreTest.java new file mode 100644 index 0000000..13c53f9 --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/messageoftheday/FileBasedMessageStoreTest.java
@@ -0,0 +1,75 @@ +// Copyright (C) 2026 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.googlesource.gerrit.plugins.messageoftheday; + +import static com.google.common.truth.Truth.assertThat; +import static java.nio.charset.StandardCharsets.UTF_8; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import org.eclipse.jgit.lib.Config; +import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.util.FS; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +public class FileBasedMessageStoreTest { + private static final String SECTION_MESSAGE = "message"; + private static final String MESSAGE_ID = "test-message"; + private static final String HTML_CONTENT = "<p>Hello World</p>"; + private static final String EXPIRES_AT = "20260323:1400"; + + @Rule public TemporaryFolder tempFolder = new TemporaryFolder(); + + private File cfgFile; + private Path dataDir; + private FileBasedMessageStore store; + + @Before + public void setUp() throws Exception { + cfgFile = tempFolder.newFile("messageoftheday.config"); + dataDir = tempFolder.newFolder("data").toPath(); + store = new FileBasedMessageStore(cfgFile, dataDir); + } + + @Test + public void writesHtmlFileWithCorrectContent() throws Exception { + Config config = new Config(); + config.setString(SECTION_MESSAGE, null, "id", MESSAGE_ID); + + store.saveConfiguredMessage(new ConfiguredMessage(config, HTML_CONTENT)); + + Path htmlFile = dataDir.resolve(MESSAGE_ID + ".html"); + assertThat(Files.exists(htmlFile)).isTrue(); + assertThat(Files.readString(htmlFile, UTF_8)).isEqualTo(HTML_CONTENT); + } + + @Test + public void writesConfigFileWithCorrectValues() throws Exception { + Config config = new Config(); + config.setString(SECTION_MESSAGE, null, "id", MESSAGE_ID); + config.setString(SECTION_MESSAGE, null, "expiresAt", EXPIRES_AT); + + store.saveConfiguredMessage(new ConfiguredMessage(config, HTML_CONTENT)); + + FileBasedConfig savedConfig = new FileBasedConfig(cfgFile, FS.DETECTED); + savedConfig.load(); + assertThat(savedConfig.getString(SECTION_MESSAGE, null, "id")).isEqualTo(MESSAGE_ID); + assertThat(savedConfig.getString(SECTION_MESSAGE, null, "expiresAt")).isEqualTo(EXPIRES_AT); + } +}