diff --git a/bukkit/rpk-chat-bukkit/src/main/kotlin/com/rpkit/chat/bukkit/command/listchatchannels/ListChatChannelsCommand.kt b/bukkit/rpk-chat-bukkit/src/main/kotlin/com/rpkit/chat/bukkit/command/listchatchannels/ListChatChannelsCommand.kt index b83d419b..2715da74 100644 --- a/bukkit/rpk-chat-bukkit/src/main/kotlin/com/rpkit/chat/bukkit/command/listchatchannels/ListChatChannelsCommand.kt +++ b/bukkit/rpk-chat-bukkit/src/main/kotlin/com/rpkit/chat/bukkit/command/listchatchannels/ListChatChannelsCommand.kt @@ -32,6 +32,7 @@ import org.bukkit.command.CommandExecutor import org.bukkit.command.CommandSender import org.bukkit.entity.Player import java.util.concurrent.CompletableFuture +import java.util.logging.Level import java.util.regex.Pattern /** @@ -68,11 +69,28 @@ class ListChatChannelsCommand(private val plugin: RPKChatBukkit) : CommandExecut // concurrently, but the lines are sent in chat channel order rather than in whichever // order the queries happen to finish in. val chatChannels = chatChannelService.chatChannels.toList() - val listenersFutures = chatChannels.map(RPKChatChannel::listeners) + // Each query is turned into a null-on-failure future before the queries are awaited + // together, so a chat channel whose listeners cannot be resolved is skipped instead of + // completing the combined future exceptionally and suppressing every other line. + val listenersFutures = chatChannels.map { chatChannel -> + chatChannel.listeners.handle { listeners, exception -> + if (exception != null) { + plugin.logger.log( + Level.SEVERE, + "Failed to get listeners for chat channel ${chatChannel.name.value}", + exception + ) + null + } else { + listeners + } + } + } CompletableFuture.allOf(*listenersFutures.toTypedArray()).thenAccept { chatChannels.forEachIndexed { index, chatChannel -> + val listeners = listenersFutures[index].join() ?: return@forEachIndexed sender.spigot().sendMessage( - *chatChannelComponents(chatChannel, listenersFutures[index].join(), minecraftProfile) + *chatChannelComponents(chatChannel, listeners, minecraftProfile) ) } } diff --git a/bukkit/rpk-chat-bukkit/src/test/kotlin/com/rpkit/chat/bukkit/test/command/listchatchannels/ListChatChannelsCommandTests.kt b/bukkit/rpk-chat-bukkit/src/test/kotlin/com/rpkit/chat/bukkit/test/command/listchatchannels/ListChatChannelsCommandTests.kt index 5b285e6f..8cacc8ae 100644 --- a/bukkit/rpk-chat-bukkit/src/test/kotlin/com/rpkit/chat/bukkit/test/command/listchatchannels/ListChatChannelsCommandTests.kt +++ b/bukkit/rpk-chat-bukkit/src/test/kotlin/com/rpkit/chat/bukkit/test/command/listchatchannels/ListChatChannelsCommandTests.kt @@ -32,11 +32,14 @@ import io.mockk.every import io.mockk.just import io.mockk.mockk import io.mockk.runs +import io.mockk.verify import net.md_5.bungee.api.chat.BaseComponent import org.bukkit.command.Command import org.bukkit.entity.Player import java.awt.Color import java.util.concurrent.CompletableFuture +import java.util.logging.Level +import java.util.logging.Logger /** * The template is deliberately free of colour codes: these tests are about which line is sent @@ -177,6 +180,100 @@ class ListChatChannelsCommandTests : WordSpec({ sentLines shouldBe emptyList() } + + "skip a chat channel whose listeners query fails and still send the rest in order" { + val messages = mockk() + every { messages["listchatchannels-title"] } returns "chat channels:" + every { messages["listchatchannels-item", any()] } returns ITEM_TEMPLATE + val logger = mockk(relaxed = true) + val plugin = mockk() + every { plugin.messages } returns messages + every { plugin.logger } returns logger + + val alphaListeners = CompletableFuture>() + val betaListeners = CompletableFuture>() + val gammaListeners = CompletableFuture>() + val chatChannelService = mockk() + every { chatChannelService.chatChannels } returns listOf( + chatChannel("alpha", alphaListeners), + chatChannel("beta", betaListeners), + chatChannel("gamma", gammaListeners) + ) + + val minecraftProfile = mockk() + every { minecraftProfile.id } returns RPKMinecraftProfileId(1) + val sender = mockk() + val senderSpigot = mockk() + val sentLines = mutableListOf() + every { sender.hasPermission("rpkit.chat.command.listchatchannels") } returns true + every { sender.sendMessage(any()) } just runs + every { sender.spigot() } returns senderSpigot + every { senderSpigot.sendMessage(*anyVararg()) } answers { + sentLines += plainText(call.invocation.args) + } + val minecraftProfileService = mockk() + every { minecraftProfileService.getPreloadedMinecraftProfile(sender) } returns minecraftProfile + val testServicesDelegate = mockk() + every { testServicesDelegate[RPKMinecraftProfileService::class.java] } returns minecraftProfileService + every { testServicesDelegate[RPKChatChannelService::class.java] } returns chatChannelService + Services.delegate = testServicesDelegate + + val listChatChannelsCommand = ListChatChannelsCommand(plugin) + listChatChannelsCommand.onCommand(sender, mockk(), "listchatchannels", emptyArray()) shouldBe true + + val betaFailure = RuntimeException("listeners query failed") + gammaListeners.complete(emptyList()) + betaListeners.completeExceptionally(betaFailure) + alphaListeners.complete(emptyList()) + + // Before this fix a single failing query completed the combined future + // exceptionally, so none of these lines was sent at all. + sentLines shouldBe listOf("- alpha (Unmute)", "- gamma (Unmute)") + verify { + logger.log(Level.SEVERE, "Failed to get listeners for chat channel beta", any()) + } + } + + "send nothing but the title when every listeners query fails" { + val messages = mockk() + every { messages["listchatchannels-title"] } returns "chat channels:" + every { messages["listchatchannels-item", any()] } returns ITEM_TEMPLATE + val plugin = mockk() + every { plugin.messages } returns messages + every { plugin.logger } returns mockk(relaxed = true) + + val alphaListeners = CompletableFuture>() + val betaListeners = CompletableFuture>() + val chatChannelService = mockk() + every { chatChannelService.chatChannels } returns listOf( + chatChannel("alpha", alphaListeners), + chatChannel("beta", betaListeners) + ) + + val sender = mockk() + val senderSpigot = mockk() + val sentLines = mutableListOf() + every { sender.hasPermission("rpkit.chat.command.listchatchannels") } returns true + every { sender.sendMessage(any()) } just runs + every { sender.spigot() } returns senderSpigot + every { senderSpigot.sendMessage(*anyVararg()) } answers { + sentLines += plainText(call.invocation.args) + } + val minecraftProfileService = mockk() + every { minecraftProfileService.getPreloadedMinecraftProfile(sender) } returns mockk() + val testServicesDelegate = mockk() + every { testServicesDelegate[RPKMinecraftProfileService::class.java] } returns minecraftProfileService + every { testServicesDelegate[RPKChatChannelService::class.java] } returns chatChannelService + Services.delegate = testServicesDelegate + + val listChatChannelsCommand = ListChatChannelsCommand(plugin) + listChatChannelsCommand.onCommand(sender, mockk(), "listchatchannels", emptyArray()) shouldBe true + + alphaListeners.completeExceptionally(RuntimeException("listeners query failed")) + betaListeners.completeExceptionally(RuntimeException("listeners query failed")) + + sentLines shouldBe emptyList() + } } })