#1265 Limbo: fallback to old "group" during deserialization, favor old limbo's groups over new limbo's

This commit is contained in:
ljacqu 2017-07-02 10:55:16 +02:00
parent c758e15cd7
commit ff99b63385
9 changed files with 55 additions and 26 deletions

View File

@ -5,7 +5,6 @@ import org.bukkit.Location;
import org.bukkit.scheduler.BukkitTask;
import java.util.Collection;
import java.util.List;
/**
* Represents a player which is not logged in and keeps track of certain states (like OP status, flying)
@ -18,14 +17,15 @@ public class LimboPlayer {
private final boolean canFly;
private final boolean operator;
private Collection<String> groups;
private final Collection<String> groups;
private final Location loc;
private final float walkSpeed;
private final float flySpeed;
private BukkitTask timeoutTask = null;
private MessageTask messageTask = null;
public LimboPlayer(Location loc, boolean operator, Collection<String> groups, boolean fly, float walkSpeed, float flySpeed) {
public LimboPlayer(Location loc, boolean operator, Collection<String> groups, boolean fly, float walkSpeed,
float flySpeed) {
this.loc = loc;
this.operator = operator;
this.groups = groups;

View File

@ -12,6 +12,8 @@ import javax.inject.Inject;
import java.util.Collection;
import java.util.Collections;
import static fr.xephi.authme.util.Utils.isCollectionEmpty;
/**
* Helper class for the LimboService.
*/
@ -70,7 +72,7 @@ class LimboServiceHelper {
* <ul>
* <li><code>isOperator, allowFlight</code>: true if either limbo has true</li>
* <li><code>flySpeed, walkSpeed</code>: maximum value of either limbo player</li>
* <li><code>group, location</code>: from old limbo if not empty/null, otherwise from new limbo</li>
* <li><code>groups, location</code>: from old limbo if not empty/null, otherwise from new limbo</li>
* </ul>
*
* @param newLimbo the new limbo player
@ -88,7 +90,7 @@ class LimboServiceHelper {
boolean canFly = newLimbo.isCanFly() || oldLimbo.isCanFly();
float flySpeed = Math.max(newLimbo.getFlySpeed(), oldLimbo.getFlySpeed());
float walkSpeed = Math.max(newLimbo.getWalkSpeed(), oldLimbo.getWalkSpeed());
Collection<String> groups = newLimbo.getGroups();
Collection<String> groups = getLimboGroups(oldLimbo.getGroups(), newLimbo.getGroups());
Location location = firstNotNull(oldLimbo.getLocation(), newLimbo.getLocation());
return new LimboPlayer(location, isOperator, groups, canFly, walkSpeed, flySpeed);
@ -97,4 +99,10 @@ class LimboServiceHelper {
private static Location firstNotNull(Location first, Location second) {
return first == null ? second : first;
}
private static Collection<String> getLimboGroups(Collection<String> oldLimboGroups,
Collection<String> newLimboGroups) {
ConsoleLogger.debug("Limbo merge: new and old groups are `{0}` and `{1}`", newLimboGroups, oldLimboGroups);
return isCollectionEmpty(oldLimboGroups) ? newLimboGroups : oldLimboGroups;
}
}

View File

@ -29,12 +29,15 @@ import static fr.xephi.authme.data.limbo.persistence.LimboPlayerSerializer.LOC_Y
import static fr.xephi.authme.data.limbo.persistence.LimboPlayerSerializer.LOC_YAW;
import static fr.xephi.authme.data.limbo.persistence.LimboPlayerSerializer.LOC_Z;
import static fr.xephi.authme.data.limbo.persistence.LimboPlayerSerializer.WALK_SPEED;
import static java.util.Optional.ofNullable;
/**
* Converts a JsonElement to a LimboPlayer.
*/
class LimboPlayerDeserializer implements JsonDeserializer<LimboPlayer> {
private static final String GROUP_LEGACY = "group";
private BukkitService bukkitService;
LimboPlayerDeserializer(BukkitService bukkitService) {
@ -51,7 +54,7 @@ class LimboPlayerDeserializer implements JsonDeserializer<LimboPlayer> {
Location loc = deserializeLocation(jsonObject);
boolean operator = getBoolean(jsonObject, IS_OP);
Collection<String> groups = getStringList(jsonObject, GROUPS);
Collection<String> groups = getLimboGroups(jsonObject);
boolean canFly = getBoolean(jsonObject, CAN_FLY);
float walkSpeed = getFloat(jsonObject, WALK_SPEED, LimboPlayer.DEFAULT_WALK_SPEED);
float flySpeed = getFloat(jsonObject, FLY_SPEED, LimboPlayer.DEFAULT_FLY_SPEED);
@ -81,10 +84,11 @@ class LimboPlayerDeserializer implements JsonDeserializer<LimboPlayer> {
return element != null ? element.getAsString() : "";
}
private static List<String> getStringList(JsonObject jsonObject, String memberName) {
JsonElement element = jsonObject.get(memberName);
private static List<String> getLimboGroups(JsonObject jsonObject) {
JsonElement element = jsonObject.get(GROUPS);
if (element == null) {
return Collections.emptyList();
String legacyGroup = ofNullable(jsonObject.get(GROUP_LEGACY)).map(JsonElement::getAsString).orElse(null);
return legacyGroup == null ? Collections.emptyList() : Collections.singletonList(legacyGroup);
}
List<String> result = new ArrayList<>();
JsonArray jsonArray = element.getAsJsonArray();

View File

@ -1,7 +1,6 @@
package fr.xephi.authme.data.limbo.persistence;
import com.google.gson.Gson;
import com.google.gson.JsonArray;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.JsonSerializationContext;
@ -10,7 +9,6 @@ import fr.xephi.authme.data.limbo.LimboPlayer;
import org.bukkit.Location;
import java.lang.reflect.Type;
import java.util.Collection;
/**
* Converts a LimboPlayer to a JsonElement.
@ -31,6 +29,8 @@ class LimboPlayerSerializer implements JsonSerializer<LimboPlayer> {
static final String WALK_SPEED = "walk-speed";
static final String FLY_SPEED = "fly-speed";
private static final Gson GSON = new Gson();
@Override
public JsonElement serialize(LimboPlayer limboPlayer, Type type, JsonSerializationContext context) {
@ -45,7 +45,7 @@ class LimboPlayerSerializer implements JsonSerializer<LimboPlayer> {
JsonObject obj = new JsonObject();
obj.add(LOCATION, locationObject);
obj.add(GROUPS, new Gson().toJsonTree(limboPlayer.getGroups()).getAsJsonArray()); //TODO: maybe we should store the GSON instance somewhere. -sg
obj.add(GROUPS, GSON.toJsonTree(limboPlayer.getGroups()).getAsJsonArray());
obj.addProperty(IS_OP, limboPlayer.isOperator());
obj.addProperty(CAN_FLY, limboPlayer.isCanFly());

View File

@ -9,6 +9,7 @@ import org.hamcrest.TypeSafeMatcher;
import java.util.Collection;
import static java.lang.String.format;
import static org.hamcrest.collection.IsIterableContainingInOrder.contains;
/**
* Contains matchers for LimboPlayer.
@ -19,17 +20,20 @@ public final class LimboPlayerMatchers {
}
public static Matcher<LimboPlayer> isLimbo(LimboPlayer limbo) {
return isLimbo(limbo.isOperator(), limbo.getGroups(), limbo.isCanFly(),
limbo.getWalkSpeed(), limbo.getFlySpeed());
String[] groups = limbo.getGroups().toArray(new String[limbo.getGroups().size()]);
return isLimbo(limbo.isOperator(), limbo.isCanFly(), limbo.getWalkSpeed(), limbo.getFlySpeed(), groups);
}
public static Matcher<LimboPlayer> isLimbo(boolean isOp, Collection<String> groups, boolean canFly,
float walkSpeed, float flySpeed) {
public static Matcher<LimboPlayer> isLimbo(boolean isOp, boolean canFly, float walkSpeed, float flySpeed,
String... groups) {
return new TypeSafeMatcher<LimboPlayer>() {
@Override
protected boolean matchesSafely(LimboPlayer item) {
return item.isOperator() == isOp && item.getGroups().equals(groups) && item.isCanFly() == canFly
&& walkSpeed == item.getWalkSpeed() && flySpeed == item.getFlySpeed();
return item.isOperator() == isOp
&& collectionContains(item.getGroups(), groups)
&& item.isCanFly() == canFly
&& walkSpeed == item.getWalkSpeed()
&& flySpeed == item.getFlySpeed();
}
@Override
@ -111,4 +115,12 @@ public final class LimboPlayerMatchers {
return hasLocation(location.getWorld().getName(), location.getX(), location.getY(), location.getZ(),
location.getYaw(), location.getPitch());
}
// Hamcrest's contains() doesn't like it when there are no items, so we need to check for the empty case explicitly
private static boolean collectionContains(Collection<String> givenItems, String... expectedItems) {
if (expectedItems.length == 0) {
return givenItems.isEmpty();
}
return contains(expectedItems).matches(givenItems);
}
}

View File

@ -8,6 +8,7 @@ import org.mockito.junit.MockitoJUnitRunner;
import java.util.Collections;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;
@ -39,7 +40,7 @@ public class LimboServiceHelperTest {
// then
assertThat(result.getLocation(), equalTo(oldLocation));
assertThat(result.isOperator(), equalTo(true));
assertThat(result.getGroups(), equalTo(Collections.singletonList("grp-new")));
assertThat(result.getGroups(), contains("grp-old"));
assertThat(result.isCanFly(), equalTo(true));
assertThat(result.getWalkSpeed(), equalTo(0.1f));
assertThat(result.getFlySpeed(), equalTo(0.8f));
@ -58,7 +59,7 @@ public class LimboServiceHelperTest {
// then
assertThat(result.getLocation(), equalTo(newLocation));
assertThat(result.isOperator(), equalTo(false));
assertThat(result.getGroups(), equalTo(Collections.singletonList("grp-new")));
assertThat(result.getGroups(), contains("grp-new"));
assertThat(result.isCanFly(), equalTo(true));
assertThat(result.getWalkSpeed(), equalTo(0.3f));
assertThat(result.getFlySpeed(), equalTo(0.1f));

View File

@ -46,22 +46,22 @@ public class DistributedFilesPersistenceHandlerTest {
/** Player is in seg32-10110 and should be migrated into seg16-f. */
private static final UUID MIGRATED_UUID = fromString("f6a97c88-7c8f-c12e-4931-6206d4ca067d");
private static final Matcher<LimboPlayer> MIGRATED_LIMBO_MATCHER =
isLimbo(false, Collections.singletonList("noob"), true, 0.2f, 0.1f);
isLimbo(false, true, 0.2f, 0.1f, "noob");
/** Existing player in seg16-f. */
private static final UUID UUID_FAB69 = fromString("fab69c88-2cd0-1fed-f00d-dead14ca067d");
private static final Matcher<LimboPlayer> FAB69_MATCHER =
isLimbo(false, Collections.emptyList(), false, 0.2f, 0.1f);
isLimbo(false, false, 0.2f, 0.1f, "");
/** Player in seg16-8. */
private static final UUID UUID_STAFF = fromString("88897c88-7c8f-c12e-4931-6206d4ca067d");
private static final Matcher<LimboPlayer> STAFF_MATCHER =
isLimbo(true, Collections.singletonList("staff"), false, 0.3f, 0.1f);
isLimbo(true, false, 0.3f, 0.1f, "staff", "mod");
/** Player in seg16-8. */
private static final UUID UUID_8C679 = fromString("8c679491-1234-abcd-9102-1fa6e0cc3f81");
private static final Matcher<LimboPlayer> SC679_MATCHER =
isLimbo(false, Collections.singletonList("primary"), true, 0.1f, 0.0f);
isLimbo(false, true, 0.1f, 0.0f, "primary");
/** UUID for which no data is stored (belongs to a segment file that does not exist, seg16-4). */
private static final UUID UNKNOWN_UUID = fromString("42d1cc0b-8f12-d04a-e7ba-a067d05cdc39");

View File

@ -23,6 +23,7 @@ import java.nio.file.Files;
import java.util.Collections;
import java.util.UUID;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
@ -79,7 +80,7 @@ public class IndividualFilesPersistenceHandlerTest {
assertThat(data.isCanFly(), equalTo(true));
assertThat(data.getWalkSpeed(), equalTo(0.2f));
assertThat(data.getFlySpeed(), equalTo(0.1f));
assertThat(data.getGroups(), equalTo(Collections.singletonList("players")));
assertThat(data.getGroups(), contains("players"));
Location location = data.getLocation();
assertThat(location.getX(), equalTo(-113.219));
assertThat(location.getY(), equalTo(72.0));

View File

@ -8,7 +8,10 @@
"yaw": 222.14977,
"pitch": 10.649977
},
"group": "staff",
"groups": [
"staff",
"mod"
],
"operator": true,
"can-fly": false,
"walk-speed": 0.3,