Fixes NPE with island ban of an offline player

https://github.com/BentoBoxWorld/BentoBox/issues/872
This commit is contained in:
tastybento 2019-08-02 09:22:12 -07:00
parent 6c0305f8f5
commit 91077ef895
2 changed files with 129 additions and 124 deletions

View File

@ -9,6 +9,7 @@ import org.bukkit.Bukkit;
import org.bukkit.Sound;
import org.bukkit.entity.Player;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable;
import world.bentobox.bentobox.api.commands.CompositeCommand;
import world.bentobox.bentobox.api.events.IslandBaseEvent;
@ -20,6 +21,8 @@ import world.bentobox.bentobox.util.Util;
public class IslandBanCommand extends CompositeCommand {
private @Nullable User target;
public IslandBanCommand(CompositeCommand islandCommand) {
super(islandCommand, "ban");
}
@ -34,7 +37,7 @@ public class IslandBanCommand extends CompositeCommand {
}
@Override
public boolean execute(User user, String label, List<String> args) {
public boolean canExecute(User user, String label, List<String> args) {
if (args.size() != 1) {
// Show help
showHelp(this, user);
@ -74,12 +77,19 @@ public class IslandBanCommand extends CompositeCommand {
if (getSettings().getBanCooldown() > 0 && checkCooldown(user, island.getUniqueId(), targetUUID.toString())) {
return false;
}
User target = User.getInstance(targetUUID);
target = User.getInstance(targetUUID);
// Cannot ban ops
if (target.hasPermission(getAddon().getPermissionPrefix() + "admin.noban")) {
if (target.isOp() || (target.isOnline() && target.hasPermission(
getAddon()
.getPermissionPrefix() + "admin.noban"))) {
user.sendMessage("commands.island.ban.cannot-ban");
return false;
}
return true;
}
@Override
public boolean execute(User user, String label, List<String> args) {
// Finished error checking - start the banning
return ban(user, target);
}

View File

@ -2,8 +2,12 @@ package world.bentobox.bentobox.api.commands.island;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.util.ArrayList;
@ -24,10 +28,11 @@ import org.bukkit.Server;
import org.bukkit.entity.Player;
import org.bukkit.plugin.PluginManager;
import org.bukkit.scheduler.BukkitScheduler;
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.stubbing.Answer;
import org.powermock.api.mockito.PowerMockito;
@ -37,6 +42,7 @@ import org.powermock.reflect.Whitebox;
import world.bentobox.bentobox.BentoBox;
import world.bentobox.bentobox.Settings;
import world.bentobox.bentobox.api.addons.Addon;
import world.bentobox.bentobox.api.commands.CompositeCommand;
import world.bentobox.bentobox.api.localization.TextVariables;
import world.bentobox.bentobox.api.user.User;
@ -44,6 +50,8 @@ import world.bentobox.bentobox.database.objects.Island;
import world.bentobox.bentobox.managers.CommandsManager;
import world.bentobox.bentobox.managers.IslandWorldManager;
import world.bentobox.bentobox.managers.IslandsManager;
import world.bentobox.bentobox.managers.LocalesManager;
import world.bentobox.bentobox.managers.PlaceholdersManager;
import world.bentobox.bentobox.managers.PlayersManager;
import world.bentobox.bentobox.managers.RanksManager;
@ -52,15 +60,25 @@ import world.bentobox.bentobox.managers.RanksManager;
*
*/
@RunWith(PowerMockRunner.class)
@PrepareForTest({Bukkit.class, BentoBox.class, User.class })
@PrepareForTest({Bukkit.class, BentoBox.class})
public class IslandBanCommandTest {
@Mock
private CompositeCommand ic;
private UUID uuid;
@Mock
private User user;
@Mock
private IslandsManager im;
@Mock
private PlayersManager pm;
@Mock
private Island island;
@Mock
private Addon addon;
private IslandBanCommand ibc;
@Mock
private Player targetPlayer;
@Before
public void setUp() throws Exception {
@ -80,26 +98,23 @@ public class IslandBanCommandTest {
// Player
Player p = mock(Player.class);
// Sometimes use Mockito.withSettings().verboseLogging()
user = mock(User.class);
when(user.isOp()).thenReturn(false);
uuid = UUID.randomUUID();
when(user.getUniqueId()).thenReturn(uuid);
when(user.getPlayer()).thenReturn(p);
when(user.getName()).thenReturn("tastybento");
when(user.getPermissionValue(anyString(), anyInt())).thenReturn(-1);
// Parent command has no aliases
ic = mock(CompositeCommand.class);
when(ic.getSubCommandAliases()).thenReturn(new HashMap<>());
// No island for player to begin with (set it later in the tests)
im = mock(IslandsManager.class);
when(im.hasIsland(Mockito.any(), Mockito.eq(uuid))).thenReturn(false);
when(im.isOwner(Mockito.any(), Mockito.eq(uuid))).thenReturn(false);
// Player has island to begin with
when(im.hasIsland(any(), eq(uuid))).thenReturn(true);
when(im.isOwner(any(), eq(uuid))).thenReturn(true);
when(plugin.getIslands()).thenReturn(im);
// Has team
pm = mock(PlayersManager.class);
when(im.inTeam(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
when(im.inTeam(any(), eq(uuid))).thenReturn(true);
when(plugin.getPlayers()).thenReturn(pm);
// Server & Scheduler
@ -108,23 +123,54 @@ public class IslandBanCommandTest {
when(Bukkit.getScheduler()).thenReturn(sch);
// Island Banned list initialization
island = mock(Island.class);
when(island.getBanned()).thenReturn(new HashSet<>());
when(island.isBanned(Mockito.any())).thenReturn(false);
when(island.getRank(Mockito.any())).thenReturn(RanksManager.OWNER_RANK);
when(im.getIsland(Mockito.any(), Mockito.any(User.class))).thenReturn(island);
when(im.getIsland(Mockito.any(), Mockito.any(UUID.class))).thenReturn(island);
when(island.isBanned(any())).thenReturn(false);
when(island.getRank(any())).thenReturn(RanksManager.OWNER_RANK);
when(im.getIsland(any(), any(User.class))).thenReturn(island);
when(im.getIsland(any(), any(UUID.class))).thenReturn(island);
// IWM friendly name
IslandWorldManager iwm = mock(IslandWorldManager.class);
when(iwm.getFriendlyName(Mockito.any())).thenReturn("BSkyBlock");
when(iwm.getFriendlyName(any())).thenReturn("BSkyBlock");
when(plugin.getIWM()).thenReturn(iwm);
// Server and Plugin Manager for events
Server server = mock(Server.class);
when(Bukkit.getServer()).thenReturn(server);
PluginManager pim = mock(PluginManager.class);
when(server.getPluginManager()).thenReturn(pim);
// Addon
when(ic.getAddon()).thenReturn(addon);
// Locales
LocalesManager lm = mock(LocalesManager.class);
when(lm.get(Mockito.any(), Mockito.any())).thenAnswer(invocation -> invocation.getArgumentAt(1, String.class));
when(plugin.getLocalesManager()).thenReturn(lm);
PlaceholdersManager phm = mock(PlaceholdersManager.class);
when(phm.replacePlaceholders(any(), any())).thenAnswer(invocation -> invocation.getArgumentAt(1, String.class));
// Placeholder manager
when(plugin.getPlaceholdersManager()).thenReturn(phm);
// Target bill - default target. Non Op, online, no ban prevention permission
UUID uuid = UUID.randomUUID();
when(pm.getUUID(Mockito.anyString())).thenReturn(uuid);
when(targetPlayer.getName()).thenReturn("bill");
when(targetPlayer.getUniqueId()).thenReturn(uuid);
when(targetPlayer.isOp()).thenReturn(false);
when(targetPlayer.isOnline()).thenReturn(true);
when(targetPlayer.hasPermission(Mockito.anyString())).thenReturn(false);
User.getInstance(targetPlayer);
// Island Ban Command
ibc = new IslandBanCommand(ic);
}
@After
public void tearDown() {
User.clearUsers();
}
/**
@ -147,168 +193,118 @@ public class IslandBanCommandTest {
@Test
public void testNoArgs() {
IslandBanCommand ibc = new IslandBanCommand(ic);
assertFalse(ibc.execute(user, ibc.getLabel(), new ArrayList<>()));
assertFalse(ibc.canExecute(user, ibc.getLabel(), new ArrayList<>()));
}
@Test
public void testNoIsland() {
when(im.inTeam(Mockito.any(), Mockito.eq(uuid))).thenReturn(false);
IslandBanCommand ibc = new IslandBanCommand(ic);
assertFalse(ibc.execute(user, ibc.getLabel(), Collections.singletonList("bill")));
Mockito.verify(user).sendMessage("general.errors.no-island");
when(im.hasIsland(any(), eq(uuid))).thenReturn(false);
when(im.isOwner(any(), eq(uuid))).thenReturn(false);
when(im.inTeam(any(), eq(uuid))).thenReturn(false);
assertFalse(ibc.canExecute(user, ibc.getLabel(), Collections.singletonList("bill")));
verify(user).sendMessage("general.errors.no-island");
}
@Test
public void testTooLowRank() {
IslandBanCommand ibc = new IslandBanCommand(ic);
when(im.hasIsland(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
when(island.getRank(Mockito.any())).thenReturn(RanksManager.MEMBER_RANK);
when(island.getRank(any())).thenReturn(RanksManager.MEMBER_RANK);
when(island.getRankCommand(anyString())).thenReturn(RanksManager.OWNER_RANK);
assertFalse(ibc.execute(user, ibc.getLabel(), Collections.singletonList("bill")));
Mockito.verify(user).sendMessage("general.errors.no-permission");
assertFalse(ibc.canExecute(user, ibc.getLabel(), Collections.singletonList("bill")));
verify(user).sendMessage("general.errors.no-permission");
}
@Test
public void testUnknownUser() {
IslandBanCommand ibc = new IslandBanCommand(ic);
when(im.hasIsland(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
when(im.isOwner(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
when(pm.getUUID(Mockito.anyString())).thenReturn(null);
assertFalse(ibc.execute(user, ibc.getLabel(), Collections.singletonList("bill")));
Mockito.verify(user).sendMessage("general.errors.unknown-player", "[name]", "bill");
assertFalse(ibc.canExecute(user, ibc.getLabel(), Collections.singletonList("bill")));
verify(user).sendMessage("general.errors.unknown-player", "[name]", "bill");
}
@Test
public void testBanSelf() {
IslandBanCommand ibc = new IslandBanCommand(ic);
when(im.hasIsland(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
when(im.isOwner(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
when(pm.getUUID(Mockito.anyString())).thenReturn(uuid);
assertFalse(ibc.execute(user, ibc.getLabel(), Collections.singletonList("bill")));
Mockito.verify(user).sendMessage("commands.island.ban.cannot-ban-yourself");
assertFalse(ibc.canExecute(user, ibc.getLabel(), Collections.singletonList("bill")));
verify(user).sendMessage("commands.island.ban.cannot-ban-yourself");
}
@Test
public void testBanTeamMate() {
IslandBanCommand ibc = new IslandBanCommand(ic);
when(im.hasIsland(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
when(im.isOwner(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
UUID teamMate = UUID.randomUUID();
when(pm.getUUID(Mockito.anyString())).thenReturn(teamMate);
Set<UUID> members = new HashSet<>();
members.add(uuid);
members.add(teamMate);
when(im.getMembers(Mockito.any(), Mockito.any())).thenReturn(members);
assertFalse(ibc.execute(user, ibc.getLabel(), Collections.singletonList("bill")));
Mockito.verify(user).sendMessage("commands.island.ban.cannot-ban-member");
when(im.getMembers(any(), any())).thenReturn(members);
assertFalse(ibc.canExecute(user, ibc.getLabel(), Collections.singletonList("bill")));
verify(user).sendMessage("commands.island.ban.cannot-ban-member");
}
@Test
public void testBanAlreadyBanned() {
IslandBanCommand ibc = new IslandBanCommand(ic);
when(im.hasIsland(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
when(im.isOwner(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
UUID bannedUser = UUID.randomUUID();
when(pm.getUUID(Mockito.anyString())).thenReturn(bannedUser);
when(island.isBanned(Mockito.eq(bannedUser))).thenReturn(true);
assertFalse(ibc.execute(user, ibc.getLabel(), Collections.singletonList("bill")));
Mockito.verify(user).sendMessage("commands.island.ban.player-already-banned");
when(island.isBanned(eq(bannedUser))).thenReturn(true);
assertFalse(ibc.canExecute(user, ibc.getLabel(), Collections.singletonList("bill")));
verify(user).sendMessage("commands.island.ban.player-already-banned");
}
@Test
@Ignore("NPE in IslandBanCommand:77")
public void testBanOp() {
IslandBanCommand ibc = new IslandBanCommand(ic);
when(im.hasIsland(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
when(im.isOwner(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
UUID op = UUID.randomUUID();
when(pm.getUUID(Mockito.anyString())).thenReturn(op);
PowerMockito.mockStatic(User.class);
User opUser = mock(User.class);
when(opUser.isOp()).thenReturn(true);
when(opUser.hasPermission(Mockito.anyString())).thenReturn(true);
when(opUser.isPlayer()).thenReturn(true);
when(User.getInstance(Mockito.any(UUID.class))).thenReturn(opUser);
assertFalse(ibc.execute(user, ibc.getLabel(), Collections.singletonList("bill")));
Mockito.verify(user).sendMessage("commands.island.ban.cannot-ban");
when(targetPlayer.isOp()).thenReturn(true);
assertFalse(ibc.canExecute(user, ibc.getLabel(), Collections.singletonList("bill")));
verify(user).sendMessage("commands.island.ban.cannot-ban");
}
@Test
@Ignore("NPE in IslandBanCommand:77")
public void testBanOfflineUser() {
IslandBanCommand ibc = new IslandBanCommand(ic);
when(im.hasIsland(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
when(im.isOwner(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
UUID targetUuid = UUID.randomUUID();
when(pm.getUUID(Mockito.anyString())).thenReturn(targetUuid);
PowerMockito.mockStatic(User.class);
User targetUser = mock(User.class);
when(targetUser.isOp()).thenReturn(false);
when(targetUser.isPlayer()).thenReturn(true);
when(targetUser.isOnline()).thenReturn(false);
when(User.getInstance(targetUuid)).thenReturn(targetUser);
when(user.getPermissionValue(Mockito.anyString(), Mockito.anyInt())).thenReturn(-1);
public void testBanOnlineNoBanPermission() {
when(targetPlayer.hasPermission(Mockito.anyString())).thenReturn(true);
User.getInstance(targetPlayer);
assertFalse(ibc.canExecute(user, ibc.getLabel(), Collections.singletonList("billy")));
verify(user).sendMessage("commands.island.ban.cannot-ban");
}
@Test
public void testBanOfflineUserSuccess() {
when(targetPlayer.isOnline()).thenReturn(false);
assertTrue(ibc.canExecute(user, ibc.getLabel(), Collections.singletonList("bill")));
// Allow adding to ban list
when(island.ban(Mockito.any(), Mockito.any())).thenReturn(true);
when(island.ban(any(), any())).thenReturn(true);
// Run execute
assertTrue(ibc.execute(user, ibc.getLabel(), Collections.singletonList("bill")));
Mockito.verify(user).sendMessage("commands.island.ban.player-banned", TextVariables.NAME, targetUser.getName());
Mockito.verify(targetUser).sendMessage("commands.island.ban.owner-banned-you", TextVariables.NAME, user.getName());
verify(user).sendMessage("commands.island.ban.player-banned", "[name]", "bill");
verify(targetPlayer).sendMessage("commands.island.ban.owner-banned-you");
}
@Test
@Ignore("NPE in IslandBanCommand:77")
public void testBanOnlineUser() {
IslandBanCommand ibc = new IslandBanCommand(ic);
when(im.hasIsland(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
when(im.isOwner(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
UUID op = UUID.randomUUID();
when(pm.getUUID(Mockito.anyString())).thenReturn(op);
PowerMockito.mockStatic(User.class);
User targetUser = mock(User.class);
when(targetUser.isOp()).thenReturn(false);
when(targetUser.isPlayer()).thenReturn(true);
when(targetUser.isOnline()).thenReturn(true);
when(User.getInstance(Mockito.any(UUID.class))).thenReturn(targetUser);
when(user.getPermissionValue(Mockito.anyString(), Mockito.anyInt())).thenReturn(-1);
public void testBanOnlineUserSuccess() {
assertTrue(ibc.canExecute(user, ibc.getLabel(), Collections.singletonList("bill")));
// Allow adding to ban list
when(island.ban(Mockito.any(), Mockito.any())).thenReturn(true);
when(island.ban(any(), any())).thenReturn(true);
assertTrue(ibc.execute(user, ibc.getLabel(), Collections.singletonList("bill")));
Mockito.verify(user).sendMessage("commands.island.ban.player-banned", TextVariables.NAME, targetUser.getName());
Mockito.verify(targetUser).sendMessage("commands.island.ban.owner-banned-you", TextVariables.NAME, user.getName());
verify(user).sendMessage("commands.island.ban.player-banned", "[name]", "bill");
verify(targetPlayer).sendMessage("commands.island.ban.owner-banned-you");
}
@Test
@Ignore("NPE in IslandBanCommand:77")
public void testCancelledBan() {
IslandBanCommand ibc = new IslandBanCommand(ic);
when(im.hasIsland(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
when(im.isOwner(Mockito.any(), Mockito.eq(uuid))).thenReturn(true);
UUID op = UUID.randomUUID();
when(pm.getUUID(Mockito.anyString())).thenReturn(op);
PowerMockito.mockStatic(User.class);
User targetUser = mock(User.class);
when(targetUser.isOp()).thenReturn(false);
when(targetUser.isPlayer()).thenReturn(true);
when(targetUser.isOnline()).thenReturn(true);
when(User.getInstance(Mockito.any(UUID.class))).thenReturn(targetUser);
// Disallow adding to ban list - even cancelled
when(island.ban(Mockito.any(), Mockito.any())).thenReturn(false);
assertTrue(ibc.canExecute(user, ibc.getLabel(), Collections.singletonList("bill")));
// Disallow adding to ban list - event cancelled
when(island.ban(any(), any())).thenReturn(false);
assertFalse(ibc.execute(user, ibc.getLabel(), Collections.singletonList("bill")));
Mockito.verify(user, Mockito.never()).sendMessage("commands.island.ban.player-banned", TextVariables.NAME, targetUser.getName());
Mockito.verify(targetUser, Mockito.never()).sendMessage("commands.island.ban.owner-banned-you", "[owner]", user.getName());
verify(user, Mockito.never()).sendMessage("commands.island.ban.player-banned", TextVariables.NAME, targetPlayer.getName());
verify(targetPlayer, Mockito.never()).sendMessage("commands.island.ban.owner-banned-you");
}
@Test
public void testTabCompleteNoIsland() {
// No island
when(im.getIsland(Mockito.any(), Mockito.any(UUID.class))).thenReturn(null);
IslandBanCommand ibc = new IslandBanCommand(ic);
when(im.getIsland(any(), any(UUID.class))).thenReturn(null);
// Set up the user
User user = mock(User.class);
when(user.getUniqueId()).thenReturn(UUID.randomUUID());
@ -352,21 +348,20 @@ public class IslandBanCommandTest {
onlinePlayers.add(p);
}
when(island.isBanned(Mockito.any(UUID.class))).thenAnswer((Answer<Boolean>) invocation -> banned.contains(invocation.getArgumentAt(0, UUID.class)));
when(island.isBanned(any(UUID.class))).thenAnswer((Answer<Boolean>) invocation -> banned.contains(invocation.getArgumentAt(0, UUID.class)));
// Create the names
when(pm.getName(Mockito.any(UUID.class))).then((Answer<String>) invocation -> online.getOrDefault(invocation.getArgumentAt(0, UUID.class), "tastybento"));
when(pm.getName(any(UUID.class))).then((Answer<String>) invocation -> online.getOrDefault(invocation.getArgumentAt(0, UUID.class), "tastybento"));
// Return a set of online players
PowerMockito.mockStatic(Bukkit.class);
when(Bukkit.getOnlinePlayers()).then((Answer<Set<Player>>) invocation -> onlinePlayers);
IslandBanCommand ibc = new IslandBanCommand(ic);
// Set up the user
User user = mock(User.class);
when(user.getUniqueId()).thenReturn(UUID.randomUUID());
Player player = mock(Player.class);
// Player can see every other player except Ian
when(player.canSee(Mockito.any(Player.class))).thenAnswer((Answer<Boolean>) invocation -> {
when(player.canSee(any(Player.class))).thenAnswer((Answer<Boolean>) invocation -> {
Player p = invocation.getArgumentAt(0, Player.class);
return !p.getName().equals("ian");
});