From 64f48909316168ecb89ebe94d25154fb0d6736b0 Mon Sep 17 00:00:00 2001 From: anthorob Date: Thu, 10 Sep 2026 05:59:55 -0400 Subject: [PATCH] Refactor settings architecture and remove singleton pattern Reworked `Settings` class to replace the static singleton pattern with constructor injection of `SettingsProvider`, improving testability and modularity. Updated all affected classes, tests, and configurations to align with the new architecture. --- .junie/plans/queue-scheduler-improvements.md | 2 +- .junie/plans/remove-settings-singleton.md | 91 +++++++++++++++++++ .../java/me/bobert/netneutrality/App.java | 38 ++++---- .../command/MessageCommandContext.java | 2 +- .../context/command/MessageMenuContext.java | 5 +- .../context/command/SlashCommandContext.java | 5 +- .../context/command/UserMenuContext.java | 5 +- .../core/context/CommandContext.java | 8 +- .../core/thread/ThreadManager.java | 12 ++- .../netneutrality/hooks/EventsHandler.java | 6 +- .../bobert/netneutrality/http/WebModule.java | 10 +- .../netneutrality/tasks/help/ReportBug.java | 9 +- .../bobert/netneutrality/utils/Settings.java | 37 +------- .../git}/ReportIssueCommandTest.java | 7 +- .../command/CommandProcessorTest.java | 26 +----- .../{feature => }/music/QueueManagerTest.java | 3 +- .../{feature => }/music/TrackInfoTest.java | 6 +- .../music/TrackSchedulerTest.java | 3 +- .../music/VoteSkipServiceTest.java | 8 +- .../source/UdioAudioSourceManagerTest.java | 3 +- .../config => utils}/SettingsTest.java | 4 +- 21 files changed, 170 insertions(+), 120 deletions(-) create mode 100644 .junie/plans/remove-settings-singleton.md rename src/test/java/me/bobert/netneutrality/{feature/git/command => command/git}/ReportIssueCommandTest.java (92%) rename src/test/java/me/bobert/netneutrality/{framework => core}/command/CommandProcessorTest.java (97%) rename src/test/java/me/bobert/netneutrality/{feature => }/music/QueueManagerTest.java (99%) rename src/test/java/me/bobert/netneutrality/{feature => }/music/TrackInfoTest.java (95%) rename src/test/java/me/bobert/netneutrality/{feature => }/music/TrackSchedulerTest.java (96%) rename src/test/java/me/bobert/netneutrality/{feature => }/music/VoteSkipServiceTest.java (95%) rename src/test/java/me/bobert/netneutrality/{feature => }/music/source/UdioAudioSourceManagerTest.java (89%) rename src/test/java/me/bobert/netneutrality/{platform/config => utils}/SettingsTest.java (68%) diff --git a/.junie/plans/queue-scheduler-improvements.md b/.junie/plans/queue-scheduler-improvements.md index 6dd10c8..58ac950 100644 --- a/.junie/plans/queue-scheduler-improvements.md +++ b/.junie/plans/queue-scheduler-improvements.md @@ -76,7 +76,7 @@ Improve the code quality, correctness, and maintainability of `QueueManager`, `T All changes in `src/main/java/me/bobert/netneutrality/music/`: - **Modified:** `QueueManager.java`, `TrackScheduler.java`, `TrackInfo.java`, `VoteSkipService.java` - **New:** `StuckTrackRecoveryHandler.java` -- **New tests:** `src/test/java/me/bobert/netneutrality/feature/music/QueueManagerTest.java`, `TrackSchedulerTest.java`, `TrackInfoTest.java`, `VoteSkipServiceTest.java` +- **New tests:** `src/test/java/me/bobert/netneutrality/command/music/QueueManagerTest.java`, `TrackSchedulerTest.java`, `TrackInfoTest.java`, `VoteSkipServiceTest.java` # Testing diff --git a/.junie/plans/remove-settings-singleton.md b/.junie/plans/remove-settings-singleton.md new file mode 100644 index 0000000..ed4f8f5 --- /dev/null +++ b/.junie/plans/remove-settings-singleton.md @@ -0,0 +1,91 @@ +--- +sessionId: session-260909-154047-1h6n +--- + +# Requirements + +### Overview & Goals +Remove the singleton pattern from `Settings` so it becomes a plain instantiable class. All access should go through constructor injection of `SettingsProvider`, except for the single bootstrap site in `App.main()` which creates the instance. + +### Scope +**In Scope:** +- Remove `static INSTANCE` field and `getInstance()` from `Settings` +- Remove static helper methods `getDiscordInvite()` and `getVersion()` from `Settings` +- Migrate all remaining `Settings.getInstance()` call sites to use injected `SettingsProvider` +- Update the test to construct `Settings` directly + +**Out of Scope:** +- Changing the `SettingsProvider` interface +- Introducing a DI framework +- Changing how properties are loaded + +# Technical Design + +### Current Implementation +- `Settings` is a lazy singleton (`getInstance()`) implementing `SettingsProvider` +- `SettingsProvider` interface already exists and is injected into most classes via `ApplicationContext` +- `App.buildContext()` already registers the Settings instance as `SettingsProvider` in the context +- **Remaining direct singleton usages (9 call sites):** + - `App.java` — `VERSION` static field (line 51), `main()` bootstrap (lines 61-62), `startup()` (line 89), `buildContext()` (line 112), `initJDA()` (line 190) + - `CommandContext.java` — `isMessageFromCreator()` (line 134) + - `WebModule.java` — `createRequest()` (line 289) via `Settings.getVersion()` + - `ReportBug.java` — `executeTask()` (line 34) via `Settings.getDiscordInvite()` + - `SettingsTest.java` — `getMavenVersion()` test (line 17) + +### Proposed Changes + +#### 1. `Settings.java` +- Make constructor `public` +- Remove `static INSTANCE` field, `getInstance()`, `getDiscordInvite()`, `getVersion()` +- Keep `getMavenVersion()` as an instance method +- Add `getDiscordInvite()` as an instance method on `SettingsProvider` (or keep it as a convenience on `Settings`) + +#### 2. `App.java` +- Create `Settings` via `new Settings()` at the top of `main()`; pass it into `App` constructor or store as a field +- Replace `VERSION` static field with an instance field initialized from the settings instance +- Thread the `SettingsProvider` through `startup()`, `buildContext()`, `initJDA()` + +#### 3. `CommandContext.java` +- Add a `SettingsProvider` field, accept it via constructor parameter +- Update `isMessageFromCreator()` to use the injected provider +- Update subclasses (`MessageCommandContext`, `SlashCommandContext`, etc.) to pass it through + +#### 4. `WebModule.java` +- Accept a version string or `SettingsProvider` in the `Builder`; replace `Settings.getVersion()` call + +#### 5. `ReportBug.java` +- Accept `SettingsProvider` via constructor injection; replace `Settings.getDiscordInvite()` with `settings.get("NETNEUTRALITY_BOT_INVITE", ...)` + +#### 6. `SettingsTest.java` +- Use `new Settings()` instead of `Settings.getInstance()` + +### Risks +- `CommandContext` is abstract with multiple subclasses — all constructors need updating +- `WebModule.Builder` is used in many places including inside `Settings.getGitVersion()` itself — the version string approach (pass a `String` rather than the full provider) keeps this simpler + +# Delivery Steps + +### ✓ Step 1: Remove singleton from Settings class +Settings can be instantiated with `new Settings()` and no longer holds static state. + +- Make the `Settings` constructor `public` +- Remove the `static INSTANCE` field and `getInstance()` method +- Remove static helpers `getDiscordInvite()` and `getVersion()` +- Move `getDiscordInvite()` logic to an instance method (or inline at call sites) +- Keep `getMavenVersion()` as-is (already instance method) + +### ✓ Step 2: Migrate App.java bootstrap to explicit instantiation +App creates and threads a single Settings instance through its lifecycle methods. + +- Create `new Settings()` at the start of `main()` and store as a field on `App` +- Replace `Settings.getInstance()` calls in `main()`, `startup()`, `buildContext()`, `initJDA()` with the field +- Replace the `VERSION` static field with an instance field or a local derived from the settings instance + +### ✓ Step 3: Inject SettingsProvider into remaining call sites +CommandContext, WebModule, and ReportBug no longer reach for the singleton. + +- Add `SettingsProvider` constructor parameter to `CommandContext`; update `isMessageFromCreator()` to use it +- Update `CommandContext` subclasses (`MessageCommandContext`, `SlashCommandContext`, etc.) to pass the provider through +- In `WebModule.Builder`, accept a version string parameter; replace `Settings.getVersion()` in `createRequest()` +- In `ReportBug`, add `SettingsProvider` constructor parameter; replace `Settings.getDiscordInvite()` with `settings.get(...)` +- Update `SettingsTest` to use `new Settings()` directly \ No newline at end of file diff --git a/src/main/java/me/bobert/netneutrality/App.java b/src/main/java/me/bobert/netneutrality/App.java index 6ef5d3c..10af23b 100644 --- a/src/main/java/me/bobert/netneutrality/App.java +++ b/src/main/java/me/bobert/netneutrality/App.java @@ -6,7 +6,6 @@ package me.bobert.netneutrality; * https://discord.gg/qUa6zNFZsd */ -import me.bobert.netneutrality.db.GSettings; import club.minnced.discord.jdave.interop.JDaveSessionFactory; import io.sentry.Sentry; import me.bobert.netneutrality.bootstrap.ApplicationContext; @@ -31,6 +30,7 @@ import me.bobert.netneutrality.db.service.GuildServiceProvider; import me.bobert.netneutrality.utils.ChannelMessagePurger; import me.bobert.netneutrality.db.JpaUtils; import me.bobert.netneutrality.utils.Settings; +import me.bobert.netneutrality.http.WebModule; import net.dv8tion.jda.api.JDA; import net.dv8tion.jda.api.JDABuilder; import net.dv8tion.jda.api.audio.AudioModuleConfig; @@ -44,22 +44,19 @@ import org.apache.logging.log4j.core.config.Configurator; public class App { private static final Logger logger = LogManager.getLogger(App.class); - /** - * Version string resolved from the early-bootstrap {@link Settings} singleton (properties must be - * readable before the {@link ApplicationContext} exists). - */ - public static final String VERSION = Settings.getVersion(); private static App instance; + private final Settings settings; private JDA jda; private final ApplicationContext context = new ApplicationContext(); static void main() { - System.out.println("Starting App v" + VERSION); + Settings settings = new Settings(); + String version = settings.getMavenVersion(); + WebModule.setDefaultVersion(version); + logger.info("Starting App v{}", version); - // Settings is intentionally a process-wide bootstrap singleton: Sentry/env/token must be - // readable before the ApplicationContext is constructed. - String env = Settings.getInstance().get("NETNEUTRALITY_BOT_ENV", "dev"); - String sentryDsn = Settings.getInstance().get("NETNEUTRALITY_SENTRY_DSN"); + String env = settings.get("NETNEUTRALITY_BOT_ENV", "dev"); + String sentryDsn = settings.get("NETNEUTRALITY_SENTRY_DSN"); if (sentryDsn != null) { logger.info("Found Sentry DSN in config, setting up the SDK.."); @@ -68,7 +65,7 @@ public class App { options.setEnvironment(env); options.setTracesSampleRate(1.0); options.setDebug(false); - options.setRelease(VERSION); + options.setRelease(version); }); } @@ -78,15 +75,16 @@ public class App { Configurator.setLevel("com.sedmelluq.discord", Level.DEBUG); } - instance = new App(); + instance = new App(settings); } - public App() { + public App(Settings settings) { + this.settings = settings; startup(); } private void startup() { - String token = Settings.getInstance().get("NETNEUTRALITY_BOT_TOKEN"); + String token = settings.get("NETNEUTRALITY_BOT_TOKEN"); if (token == null) { throw new RuntimeException("No bot token provided, exiting."); } @@ -109,10 +107,8 @@ public class App { * context stops components in the exact reverse order. */ private void buildContext() { - SettingsProvider settings = Settings.getInstance(); - // Config first so SettingsProvider is resolvable for constructor injection. - context.register(settings); + context.register((SettingsProvider) settings); // Allow ApplicationContext itself to be injected into components that need it. context.register(context); @@ -187,7 +183,7 @@ public class App { } public void initJDA() { - String token = Settings.getInstance().get("NETNEUTRALITY_BOT_TOKEN"); + String token = settings.get("NETNEUTRALITY_BOT_TOKEN"); if (token == null) { SessionManager sessions = context.get(SessionManager.class); if (sessions != null) { @@ -216,9 +212,9 @@ public class App { } public void shutdown() { - System.out.println("[Net Neutrality] Shutting down..."); + logger.info("[Net Neutrality] Shutting down..."); context.stop(); - System.out.println("[Net Neutrality] Shutdown complete"); + logger.info("[Net Neutrality] Shutdown complete"); } public JDA getJDA() { diff --git a/src/main/java/me/bobert/netneutrality/context/command/MessageCommandContext.java b/src/main/java/me/bobert/netneutrality/context/command/MessageCommandContext.java index 80a4081..be0b6b2 100644 --- a/src/main/java/me/bobert/netneutrality/context/command/MessageCommandContext.java +++ b/src/main/java/me/bobert/netneutrality/context/command/MessageCommandContext.java @@ -33,7 +33,7 @@ public class MessageCommandContext extends CommandContext implements MessageCont private final SettingsProvider settings; public MessageCommandContext(MessageReceivedEvent event, SettingsProvider settings) { - super(event.getJDA(), event.isFromGuild() ? event.getGuild() : null, event.getChannel(), event.getMember(), event.getAuthor()); + super(event.getJDA(), event.isFromGuild() ? event.getGuild() : null, event.getChannel(), event.getMember(), event.getAuthor(), settings); this.settings = settings; this.event = event; this.message = event.getMessage(); diff --git a/src/main/java/me/bobert/netneutrality/context/command/MessageMenuContext.java b/src/main/java/me/bobert/netneutrality/context/command/MessageMenuContext.java index 89e2d6b..5cbce78 100644 --- a/src/main/java/me/bobert/netneutrality/context/command/MessageMenuContext.java +++ b/src/main/java/me/bobert/netneutrality/context/command/MessageMenuContext.java @@ -9,6 +9,7 @@ package me.bobert.netneutrality.context.command; import me.bobert.netneutrality.core.command.options.CommandOption; import me.bobert.netneutrality.core.context.CommandContext; import me.bobert.netneutrality.core.context.InteractionContext; +import me.bobert.netneutrality.utils.SettingsProvider; import net.dv8tion.jda.api.entities.Message; import net.dv8tion.jda.api.entities.channel.ChannelType; import net.dv8tion.jda.api.events.Event; @@ -20,8 +21,8 @@ import java.util.List; public class MessageMenuContext extends CommandContext implements InteractionContext { private final MessageContextInteractionEvent event; - public MessageMenuContext(MessageContextInteractionEvent event) { - super(event.getJDA(), event.getGuild(), event.getChannel(), event.getMember(), event.getUser()); + public MessageMenuContext(MessageContextInteractionEvent event, SettingsProvider settings) { + super(event.getJDA(), event.getGuild(), event.getChannel(), event.getMember(), event.getUser(), settings); this.event = event; } diff --git a/src/main/java/me/bobert/netneutrality/context/command/SlashCommandContext.java b/src/main/java/me/bobert/netneutrality/context/command/SlashCommandContext.java index 2d81b39..d5a381f 100644 --- a/src/main/java/me/bobert/netneutrality/context/command/SlashCommandContext.java +++ b/src/main/java/me/bobert/netneutrality/context/command/SlashCommandContext.java @@ -9,6 +9,7 @@ import me.bobert.netneutrality.core.command.options.CommandOption; import me.bobert.netneutrality.core.command.options.SlashCommandOption; import me.bobert.netneutrality.core.context.CommandContext; import me.bobert.netneutrality.core.context.InteractionContext; +import me.bobert.netneutrality.utils.SettingsProvider; import net.dv8tion.jda.api.entities.channel.ChannelType; import net.dv8tion.jda.api.events.interaction.command.SlashCommandInteractionEvent; import net.dv8tion.jda.api.interactions.commands.build.OptionData; @@ -19,8 +20,8 @@ import java.util.stream.Collectors; public class SlashCommandContext extends CommandContext implements InteractionContext { private final SlashCommandInteractionEvent event; - public SlashCommandContext(SlashCommandInteractionEvent event) { - super(event.getJDA(), event.getGuild(), event.getChannel(), event.getMember(), event.getUser()); + public SlashCommandContext(SlashCommandInteractionEvent event, SettingsProvider settings) { + super(event.getJDA(), event.getGuild(), event.getChannel(), event.getMember(), event.getUser(), settings); this.event = event; } diff --git a/src/main/java/me/bobert/netneutrality/context/command/UserMenuContext.java b/src/main/java/me/bobert/netneutrality/context/command/UserMenuContext.java index b3342ed..4f8722e 100644 --- a/src/main/java/me/bobert/netneutrality/context/command/UserMenuContext.java +++ b/src/main/java/me/bobert/netneutrality/context/command/UserMenuContext.java @@ -9,6 +9,7 @@ package me.bobert.netneutrality.context.command; import me.bobert.netneutrality.core.command.options.CommandOption; import me.bobert.netneutrality.core.context.CommandContext; import me.bobert.netneutrality.core.context.InteractionContext; +import me.bobert.netneutrality.utils.SettingsProvider; import net.dv8tion.jda.api.entities.User; import net.dv8tion.jda.api.entities.channel.ChannelType; import net.dv8tion.jda.api.entities.channel.middleman.MessageChannel; @@ -21,8 +22,8 @@ import java.util.List; public class UserMenuContext extends CommandContext implements InteractionContext { private final UserContextInteractionEvent event; - public UserMenuContext(UserContextInteractionEvent event) { - super(event.getJDA(), event.getGuild(), (MessageChannel) event.getChannel(), event.getMember(), event.getUser()); + public UserMenuContext(UserContextInteractionEvent event, SettingsProvider settings) { + super(event.getJDA(), event.getGuild(), (MessageChannel) event.getChannel(), event.getMember(), event.getUser(), settings); this.event = event; } diff --git a/src/main/java/me/bobert/netneutrality/core/context/CommandContext.java b/src/main/java/me/bobert/netneutrality/core/context/CommandContext.java index 55983d2..76abefa 100644 --- a/src/main/java/me/bobert/netneutrality/core/context/CommandContext.java +++ b/src/main/java/me/bobert/netneutrality/core/context/CommandContext.java @@ -7,7 +7,7 @@ package me.bobert.netneutrality.core.context; import me.bobert.netneutrality.core.command.Command; import me.bobert.netneutrality.core.command.options.CommandOption; -import me.bobert.netneutrality.utils.Settings; +import me.bobert.netneutrality.utils.SettingsProvider; import net.dv8tion.jda.api.JDA; import net.dv8tion.jda.api.entities.Guild; import net.dv8tion.jda.api.entities.Member; @@ -33,10 +33,12 @@ public abstract class CommandContext extends AbstractContext { private SubcommandData subcommand; private final Map invalidOptions = new HashMap<>(); private final List options; + private final SettingsProvider settings; - public CommandContext(JDA jda, Guild guild, MessageChannel channel, Member member, User user) { + public CommandContext(JDA jda, Guild guild, MessageChannel channel, Member member, User user, SettingsProvider settings) { super(jda, guild, channel, member, user); this.options = new ArrayList<>(); + this.settings = settings; } public Command getCommand() { @@ -131,7 +133,7 @@ public abstract class CommandContext extends AbstractContext { } public boolean isMessageFromCreator() { - String creatorId = Settings.getInstance().get("NETNEUTRALITY_BOT_OWNER"); + String creatorId = settings.get("NETNEUTRALITY_BOT_OWNER"); return creatorId.equals(getDiscordUser().getId()); } diff --git a/src/main/java/me/bobert/netneutrality/core/thread/ThreadManager.java b/src/main/java/me/bobert/netneutrality/core/thread/ThreadManager.java index 4e56d67..6340c0e 100644 --- a/src/main/java/me/bobert/netneutrality/core/thread/ThreadManager.java +++ b/src/main/java/me/bobert/netneutrality/core/thread/ThreadManager.java @@ -5,6 +5,9 @@ package me.bobert.netneutrality.core.thread; * https://discord.gg/qUa6zNFZsd */ +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; @@ -18,6 +21,7 @@ import java.util.concurrent.TimeUnit; */ public class ThreadManager implements ExecutorProvider { + private static final Logger logger = LogManager.getLogger(ThreadManager.class); private final Map executors = new ConcurrentHashMap<>(); public ThreadManager() { @@ -56,7 +60,7 @@ public class ThreadManager implements ExecutorProvider { @Override public void stop() { - System.out.println("[Executor] Shutting down..."); + logger.info("[Executor] Shutting down..."); try { for (ExecutorService ex : executors.values()) { @@ -64,17 +68,17 @@ public class ThreadManager implements ExecutorProvider { ex.awaitTermination(1, TimeUnit.SECONDS); } } catch (InterruptedException e) { - System.err.println("[Executor] Running tasks interrupted"); + logger.info("[Executor] Running tasks interrupted"); } finally { executors.forEach((name, ex) -> { if (!ex.isTerminated()) { - System.err.println("[Executor: " + name + "] Forcing shutdown of running tasks.."); + logger.warn("[Executor: {}] Forcing shutdown of running tasks..", name); ex.shutdownNow(); } }); } - System.out.println("[Executor] Shutdown finished!"); + logger.info("[Executor] Shutdown finished!"); } } diff --git a/src/main/java/me/bobert/netneutrality/hooks/EventsHandler.java b/src/main/java/me/bobert/netneutrality/hooks/EventsHandler.java index c129361..7c13a12 100644 --- a/src/main/java/me/bobert/netneutrality/hooks/EventsHandler.java +++ b/src/main/java/me/bobert/netneutrality/hooks/EventsHandler.java @@ -88,9 +88,9 @@ public class EventsHandler implements EventListener { return new MessageTaskContext(event); }); - addListener(UserContextInteractionEvent.class, UserMenuContext::new); - addListener(MessageContextInteractionEvent.class, MessageMenuContext::new); - addListener(SlashCommandInteractionEvent.class, SlashCommandContext::new); + addListener(UserContextInteractionEvent.class, event -> new UserMenuContext(event, settings)); + addListener(MessageContextInteractionEvent.class, event -> new MessageMenuContext(event, settings)); + addListener(SlashCommandInteractionEvent.class, event -> new SlashCommandContext(event, settings)); addListener(ModalInteractionEvent.class, ModalTaskContext::new); addListener(ButtonInteractionEvent.class, ButtonTaskContext::new); addListener(MessageReactionAddEvent.class, EmojiTaskContext::new); diff --git a/src/main/java/me/bobert/netneutrality/http/WebModule.java b/src/main/java/me/bobert/netneutrality/http/WebModule.java index 5723a67..13b3dc9 100644 --- a/src/main/java/me/bobert/netneutrality/http/WebModule.java +++ b/src/main/java/me/bobert/netneutrality/http/WebModule.java @@ -9,7 +9,6 @@ package me.bobert.netneutrality.http; import com.google.gson.Gson; import io.sentry.Breadcrumb; import io.sentry.Sentry; -import me.bobert.netneutrality.utils.Settings; import okhttp3.Headers; import okhttp3.MediaType; import okhttp3.OkHttpClient; @@ -35,7 +34,12 @@ public class WebModule { } private static final String USER_AGENT = "Net Neutrality/%s (Discord bot, discord.gg/trAqhQ5Wsr)"; + private static volatile String defaultVersion = "0.0.0"; private static final Logger log = LoggerFactory.getLogger(WebModule.class); + + public static void setDefaultVersion(String version) { + defaultVersion = version; + } private static final Map cache = Collections.synchronizedMap( new LinkedHashMap<>(100, 0.75f, true) { @Override @@ -170,6 +174,7 @@ public class WebModule { public static class Builder { private Request request; private final String url; + private String version; private Method method = Method.GET; private long timeout = 5; private TimeUnit timeUnit = TimeUnit.MINUTES; @@ -187,6 +192,7 @@ public class WebModule { */ public Builder(String url) { this.url = url; + this.version = defaultVersion; } public Builder ping() { @@ -286,7 +292,7 @@ public class WebModule { private Request createRequest() { Request.Builder builder = new Request.Builder().url(url); - builder.addHeader("User-Agent", String.format(USER_AGENT, Settings.getVersion())); + builder.addHeader("User-Agent", String.format(USER_AGENT, version)); headers.forEach(builder::addHeader); if (method == Method.PING) { diff --git a/src/main/java/me/bobert/netneutrality/tasks/help/ReportBug.java b/src/main/java/me/bobert/netneutrality/tasks/help/ReportBug.java index fc0b44e..72beb1b 100644 --- a/src/main/java/me/bobert/netneutrality/tasks/help/ReportBug.java +++ b/src/main/java/me/bobert/netneutrality/tasks/help/ReportBug.java @@ -9,13 +9,16 @@ import me.bobert.netneutrality.core.context.TaskContext; import me.bobert.netneutrality.core.tasks.impl.AbstractBuiltInTask; import me.bobert.netneutrality.core.tasks.TaskManager; import me.bobert.netneutrality.core.tasks.trait.WithButton; -import me.bobert.netneutrality.utils.Settings; +import me.bobert.netneutrality.utils.SettingsProvider; import net.dv8tion.jda.api.components.buttons.Button; public class ReportBug extends AbstractBuiltInTask implements WithButton { - public ReportBug(TaskManager taskManager) { + private final SettingsProvider settings; + + public ReportBug(TaskManager taskManager, SettingsProvider settings) { super(taskManager); + this.settings = settings; } @Override @@ -31,6 +34,6 @@ public class ReportBug extends AbstractBuiltInTask implements WithButton { @Override public void executeTask(TaskContext context) { context.replyEphemeral("To report a bug with the bot, you can either use the /report-bug command " + - "or join the discord here: " + Settings.getDiscordInvite()); + "or join the discord here: " + settings.get("NETNEUTRALITY_BOT_INVITE", "https://discord.gg/PyCpd7JGMp")); } } diff --git a/src/main/java/me/bobert/netneutrality/utils/Settings.java b/src/main/java/me/bobert/netneutrality/utils/Settings.java index 3d2595b..b429859 100644 --- a/src/main/java/me/bobert/netneutrality/utils/Settings.java +++ b/src/main/java/me/bobert/netneutrality/utils/Settings.java @@ -20,19 +20,14 @@ import java.util.concurrent.TimeUnit; /** * Process-wide configuration loaded from {@code bot.properties} and environment variables. - * - *

{@link #getInstance()} is retained intentionally as the early-bootstrap access point: Sentry, - * env and the bot token must be readable before the {@code ApplicationContext} exists. Once the - * context is built, prefer injecting {@link SettingsProvider}. */ public class Settings implements SettingsProvider { - private static Settings INSTANCE; private final File propFile = new File("bot.properties"); private final Properties prop = new Properties(); public final String DATETIMEFORMAT = "yyyy-MM-dd HH:mm:ss"; - private Settings() { + public Settings() { checkAndLoadPropertiesFile(); loadEnvironmentVariables(); } @@ -53,36 +48,6 @@ public class Settings implements SettingsProvider { } } - /** - * Early-bootstrap accessor used before the application context exists. - * - * @return the process-wide settings instance - */ - public static Settings getInstance() { - if (INSTANCE == null) { - INSTANCE = new Settings(); - } - - return INSTANCE; - } - - /** - * Bootstrap helper for the public Discord invite URL. - * - * @return the invite URL - */ - public static String getDiscordInvite() { - return Settings.getInstance().get("NETNEUTRALITY_BOT_INVITE", "https://discord.gg/PyCpd7JGMp"); - } - - /** - * Bootstrap helper for the bot version string. - * - * @return the version - */ - public static String getVersion() { - return Settings.getInstance().getMavenVersion(); - } @Override public String get(String key) { diff --git a/src/test/java/me/bobert/netneutrality/feature/git/command/ReportIssueCommandTest.java b/src/test/java/me/bobert/netneutrality/command/git/ReportIssueCommandTest.java similarity index 92% rename from src/test/java/me/bobert/netneutrality/feature/git/command/ReportIssueCommandTest.java rename to src/test/java/me/bobert/netneutrality/command/git/ReportIssueCommandTest.java index 991d621..a21ae5f 100644 --- a/src/test/java/me/bobert/netneutrality/feature/git/command/ReportIssueCommandTest.java +++ b/src/test/java/me/bobert/netneutrality/command/git/ReportIssueCommandTest.java @@ -14,11 +14,10 @@ import org.junit.jupiter.api.Test; import java.util.UUID; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; class ReportIssueCommandTest { private GitLabApi mockGitLabApi; diff --git a/src/test/java/me/bobert/netneutrality/framework/command/CommandProcessorTest.java b/src/test/java/me/bobert/netneutrality/core/command/CommandProcessorTest.java similarity index 97% rename from src/test/java/me/bobert/netneutrality/framework/command/CommandProcessorTest.java rename to src/test/java/me/bobert/netneutrality/core/command/CommandProcessorTest.java index e4caaa4..4d7108f 100644 --- a/src/test/java/me/bobert/netneutrality/framework/command/CommandProcessorTest.java +++ b/src/test/java/me/bobert/netneutrality/core/command/CommandProcessorTest.java @@ -10,8 +10,7 @@ import me.bobert.netneutrality.context.command.MessageCommandContext; import me.bobert.netneutrality.core.command.impl.AbstractCommand; import me.bobert.netneutrality.core.command.options.CommandOption; import me.bobert.netneutrality.core.context.CommandContext; -import me.bobert.netneutrality.utils.Settings; -import me.bobert.netneutrality.core.tasks.TaskManagerImpl; +import me.bobert.netneutrality.utils.SettingsProvider; import me.bobert.netneutrality.core.tasks.TaskManager; import net.dv8tion.jda.api.JDA; import net.dv8tion.jda.api.entities.*; @@ -24,14 +23,12 @@ import net.dv8tion.jda.api.interactions.commands.build.OptionData; import net.dv8tion.jda.api.interactions.commands.build.SubcommandData; import net.dv8tion.jda.api.interactions.commands.build.SubcommandGroupData; import net.dv8tion.jda.api.utils.messages.MessageCreateData; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import me.bobert.netneutrality.bootstrap.ApplicationContext; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; import org.mockito.Mock; -import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; import java.util.Arrays; @@ -65,8 +62,8 @@ import static org.mockito.Mockito.*; class CommandProcessorTest { // ----- Test Command Implementation ----- private static class TestCommand extends AbstractCommand { - static Consumer optionsConfigurator = builder -> {}; - static Consumer onExecute = ctx -> {}; + static Consumer optionsConfigurator = _ -> {}; + static Consumer onExecute = _ -> {}; @Override public String getCommandName() { @@ -229,15 +226,13 @@ class CommandProcessorTest { private Mentions mentions; @Mock - private Settings settings; + private SettingsProvider settings; private CommandManager commandManager; private TaskManager taskManager; - private MockedStatic settingsMockedStatic; - private void registerTestCommand(Consumer optionsConfig) { TestCommand.optionsConfigurator = optionsConfig; - TestCommand.onExecute = ctx -> {}; // Reset executor + TestCommand.onExecute = _ -> {}; // Reset executor Command command = new TestCommand(); CommandContainer container = new CommandContainer(command); commandManager.getCommands().put(command.getCommandName(), container); @@ -297,23 +292,12 @@ class CommandProcessorTest { @BeforeEach void setup() { - // Mock Settings to return "$" as prefix (still used by message-context prefix checks) - settingsMockedStatic = mockStatic(Settings.class); - settingsMockedStatic.when(Settings::getInstance).thenReturn(settings); when(settings.getPrefix()).thenReturn("$"); commandManager = new CommandManager(mock(ApplicationContext.class)); taskManager = mock(TaskManager.class); } - @AfterEach - void teardown() { - if (settingsMockedStatic != null) { - settingsMockedStatic.close(); - settingsMockedStatic = null; - } - } - // ===== Subcommand / Subcommand Group Tests ===== @Test diff --git a/src/test/java/me/bobert/netneutrality/feature/music/QueueManagerTest.java b/src/test/java/me/bobert/netneutrality/music/QueueManagerTest.java similarity index 99% rename from src/test/java/me/bobert/netneutrality/feature/music/QueueManagerTest.java rename to src/test/java/me/bobert/netneutrality/music/QueueManagerTest.java index 862a186..ddc8889 100644 --- a/src/test/java/me/bobert/netneutrality/feature/music/QueueManagerTest.java +++ b/src/test/java/me/bobert/netneutrality/music/QueueManagerTest.java @@ -1,4 +1,4 @@ -package me.bobert.netneutrality.feature.music; +package me.bobert.netneutrality.music; /* * Copyright (c) anthorob * @@ -9,7 +9,6 @@ import com.sedmelluq.discord.lavaplayer.player.AudioPlayer; import com.sedmelluq.discord.lavaplayer.track.AudioTrack; import com.sedmelluq.discord.lavaplayer.track.AudioTrackInfo; import me.bobert.netneutrality.core.command.CommandExecutionResult; -import me.bobert.netneutrality.music.*; import net.dv8tion.jda.api.entities.Guild; import net.dv8tion.jda.api.entities.Member; import org.junit.jupiter.api.BeforeEach; diff --git a/src/test/java/me/bobert/netneutrality/feature/music/TrackInfoTest.java b/src/test/java/me/bobert/netneutrality/music/TrackInfoTest.java similarity index 95% rename from src/test/java/me/bobert/netneutrality/feature/music/TrackInfoTest.java rename to src/test/java/me/bobert/netneutrality/music/TrackInfoTest.java index e2b1b19..fe0a22c 100644 --- a/src/test/java/me/bobert/netneutrality/feature/music/TrackInfoTest.java +++ b/src/test/java/me/bobert/netneutrality/music/TrackInfoTest.java @@ -1,4 +1,4 @@ -package me.bobert.netneutrality.feature.music; +package me.bobert.netneutrality.music; /* * Copyright (c) anthorob * @@ -6,7 +6,6 @@ package me.bobert.netneutrality.feature.music; */ import com.sedmelluq.discord.lavaplayer.track.AudioTrack; -import me.bobert.netneutrality.music.TrackInfo; import net.dv8tion.jda.api.entities.Member; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -14,7 +13,8 @@ import org.junit.jupiter.api.Test; import java.util.Set; import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class TrackInfoTest { diff --git a/src/test/java/me/bobert/netneutrality/feature/music/TrackSchedulerTest.java b/src/test/java/me/bobert/netneutrality/music/TrackSchedulerTest.java similarity index 96% rename from src/test/java/me/bobert/netneutrality/feature/music/TrackSchedulerTest.java rename to src/test/java/me/bobert/netneutrality/music/TrackSchedulerTest.java index fdd025d..676f915 100644 --- a/src/test/java/me/bobert/netneutrality/feature/music/TrackSchedulerTest.java +++ b/src/test/java/me/bobert/netneutrality/music/TrackSchedulerTest.java @@ -1,4 +1,4 @@ -package me.bobert.netneutrality.feature.music; +package me.bobert.netneutrality.music; /* * Copyright (c) anthorob * @@ -8,7 +8,6 @@ package me.bobert.netneutrality.feature.music; import com.sedmelluq.discord.lavaplayer.player.AudioPlayer; import com.sedmelluq.discord.lavaplayer.track.AudioTrack; import com.sedmelluq.discord.lavaplayer.track.AudioTrackEndReason; -import me.bobert.netneutrality.music.*; import net.dv8tion.jda.api.entities.Guild; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; diff --git a/src/test/java/me/bobert/netneutrality/feature/music/VoteSkipServiceTest.java b/src/test/java/me/bobert/netneutrality/music/VoteSkipServiceTest.java similarity index 95% rename from src/test/java/me/bobert/netneutrality/feature/music/VoteSkipServiceTest.java rename to src/test/java/me/bobert/netneutrality/music/VoteSkipServiceTest.java index 6d809ed..87f7627 100644 --- a/src/test/java/me/bobert/netneutrality/feature/music/VoteSkipServiceTest.java +++ b/src/test/java/me/bobert/netneutrality/music/VoteSkipServiceTest.java @@ -1,4 +1,4 @@ -package me.bobert.netneutrality.feature.music; +package me.bobert.netneutrality.music; /* * Copyright (c) anthorob * @@ -7,9 +7,6 @@ package me.bobert.netneutrality.feature.music; import com.sedmelluq.discord.lavaplayer.track.AudioTrack; import me.bobert.netneutrality.core.command.CommandExecutionResult; -import me.bobert.netneutrality.music.QueueManager; -import me.bobert.netneutrality.music.TrackInfo; -import me.bobert.netneutrality.music.VoteSkipService; import me.bobert.netneutrality.core.context.CommandContext; import net.dv8tion.jda.api.entities.Guild; import net.dv8tion.jda.api.entities.Member; @@ -19,7 +16,8 @@ import org.junit.jupiter.api.Test; import java.util.List; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.*; public class VoteSkipServiceTest { diff --git a/src/test/java/me/bobert/netneutrality/feature/music/source/UdioAudioSourceManagerTest.java b/src/test/java/me/bobert/netneutrality/music/source/UdioAudioSourceManagerTest.java similarity index 89% rename from src/test/java/me/bobert/netneutrality/feature/music/source/UdioAudioSourceManagerTest.java rename to src/test/java/me/bobert/netneutrality/music/source/UdioAudioSourceManagerTest.java index 438a1db..ff79742 100644 --- a/src/test/java/me/bobert/netneutrality/feature/music/source/UdioAudioSourceManagerTest.java +++ b/src/test/java/me/bobert/netneutrality/music/source/UdioAudioSourceManagerTest.java @@ -10,7 +10,8 @@ import com.sedmelluq.discord.lavaplayer.track.AudioReference; import com.sedmelluq.discord.lavaplayer.track.AudioTrackInfo; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; public class UdioAudioSourceManagerTest { diff --git a/src/test/java/me/bobert/netneutrality/platform/config/SettingsTest.java b/src/test/java/me/bobert/netneutrality/utils/SettingsTest.java similarity index 68% rename from src/test/java/me/bobert/netneutrality/platform/config/SettingsTest.java rename to src/test/java/me/bobert/netneutrality/utils/SettingsTest.java index 076025a..580b0de 100644 --- a/src/test/java/me/bobert/netneutrality/platform/config/SettingsTest.java +++ b/src/test/java/me/bobert/netneutrality/utils/SettingsTest.java @@ -8,13 +8,13 @@ package me.bobert.netneutrality.utils; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertNotNull; public class SettingsTest { @Test public void getMavenVersion() { - String version = Settings.getInstance().getMavenVersion(); + String version = new Settings().getMavenVersion(); assertNotNull(version); }