From cf7045452dea60a9fd9b38e1c210f3d68dd733e7 Mon Sep 17 00:00:00 2001 From: Susucre <34709007+Susucre@users.noreply.github.com> Date: Wed, 30 Aug 2023 23:57:12 +0200 Subject: [PATCH 1/2] Test: validate that view data is meant to be sent. --- .../test/java/mage/view/ValidateViews.java | 184 ++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 Mage.Common/src/test/java/mage/view/ValidateViews.java diff --git a/Mage.Common/src/test/java/mage/view/ValidateViews.java b/Mage.Common/src/test/java/mage/view/ValidateViews.java new file mode 100644 index 00000000000..0f992192a05 --- /dev/null +++ b/Mage.Common/src/test/java/mage/view/ValidateViews.java @@ -0,0 +1,184 @@ +package mage.view; + +import org.checkerframework.checker.units.qual.C; +import org.junit.Assert; +import org.junit.Test; + +import java.io.Serializable; +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.util.*; + +public class ValidateViews { + + // All those class are sent to client as Views or inside Views. + // The test in this file is attempting to find any class that should + // not be sent. + private static List> toValidate = Arrays.asList( + // Views + AbilityPickerView.class, AbilityView.class, CardsView.class, + CardView.class, ChatMessage.class, CombatGroupView.class, + CommanderView.class, CommandObjectView.class, CounterView.class, + DeckView.class, DraftClientMessage.class, DraftPickView.class, + DraftView.class, DungeonView.class, EmblemView.class, + ExileView.class, GameClientMessage.class, GameEndView.class, + GameTypeView.class, GameView.class, LookedAtView.class, + ManaPoolView.class, MatchView.class, PermanentView.class, + PermanentView.class, PlaneView.class, PlayerView.class, + RevealedView.class, RoomUsersView.class, RoundView.class, + SeatView.class, SelectableObjectView.class, SimpleCardsView.class, + SimpleCardView.class, StackAbilityView.class, TableClientMessage.class, + TableView.class, TournamentGameView.class, TournamentPlayerView.class, + TournamentTypeView.class, TournamentView.class, UserDataView.class, + UserRequestMessage.class, UsersView.class, UserView.class, + + ChatMessage.MessageColor.class, + ChatMessage.SoundToPlay.class, + ChatMessage.MessageType.class, + + // From mage + mage.constants.MageObjectType.class, + mage.constants.AbilityType.class, + mage.players.PlayerType.class, + + mage.MageInt.class, + mage.ObjectColor.class, + + mage.constants.CardType.class, + mage.constants.CardType.CardTypePredicate.class, + mage.constants.SuperType.class, + mage.constants.SuperType.SuperTypePredicate.class, + mage.constants.SubType.class, + mage.constants.SubType.SubTypePredicate.class, + mage.constants.SubTypeSet.class, + mage.util.SubTypes.class, + + mage.constants.Rarity.class, + + mage.constants.Zone.class, + + mage.constants.PhaseStep.class, + mage.constants.TurnPhase.class, + + mage.players.PlayableObjectStats.class, + mage.players.PlayableObjectsList.class, + + mage.counters.Counters.class, + mage.counters.Counter.class, + mage.filter.FilterMana.class, + + mage.abilities.icon.CardIcon.class, + + mage.cards.ArtRect.class, + java.awt.geom.Rectangle2D.class, + mage.cards.FrameStyle.class, + mage.cards.FrameStyle.BorderType.class, + + mage.choices.Choice.class, + mage.util.MultiAmountMessage.class, + mage.constants.PlayerAction.class, + + mage.constants.SkillLevel.class, + mage.constants.TableState.class, + + mage.players.net.UserData.class, + mage.players.net.UserSkipPrioritySteps.class, + mage.players.net.SkipPrioritySteps.class + ); + + // Those are safe java class we allow in Views. + private static List> safeClass = Arrays.asList( + boolean.class, int.class, long.class, float.class, + String.class, Date.class, UUID.class + ); + + // Those are non-public class + private static List nonPublicNames = Arrays.asList( + "mage.players.PlayableObjectRecord" + ); + + // Those are excluded as Reflection on type of Map/Set/Collection is really hard. + private static List tooHardToRecurseInto = Arrays.asList( + "mage.view.CardsView", // + "mage.view.CardsView", // + "mage.view.CardsView", // + "mage.view.ExileView", // + "mage.view.ExileView", // + "mage.view.ExileView", // + "mage.view.SimpleCardsView", // + "mage.view.SimpleCardsView", // + "mage.view.SimpleCardsView", // + + "mage.counters.Counters", // + "mage.counters.Counters", // + + "mage.util.SubTypes" // + ); + + // Same, but for fields. + private static List tooHardToRecurseIntoField = Arrays.asList( + "mage.view.GameClientMessage: ::targets" // + ); + + @Test + public void validateViewsContainSafeClass() { + for (Class clazz : toValidate) { + validateView(clazz); + } + } + + private void validateView(Class clazz) { + + Class subClass = clazz; + while (subClass != Object.class && subClass != null) { + for (Field field : subClass.getDeclaredFields()) { + if (field.getName().startsWith("$VALUES")) { + // enum values. + continue; + } + if (Modifier.isStatic(field.getModifiers())) { + continue; + } + if (tooHardToRecurseInto.contains(clazz.getName() + "<" + subClass.getName() + ">")) { + continue; + } + expectedClass(field.getGenericType(), field.getType(), clazz.getName() + ": <" + subClass.getName() + ">::" + field.getName()); + } + + subClass = subClass.getSuperclass(); + } + } + + private void expectedClass(Type type, Class clazz, String msg) { + if (toValidate.contains(clazz)) { + return; + } + if (safeClass.contains(clazz)) { + return; + } + if (nonPublicNames.contains(clazz.getName())) { + return; + } + if (clazz.equals(List.class) || clazz.equals(ArrayList.class) || clazz.equals(Map.class)) { + ParameterizedType paramType = (ParameterizedType) type; + if (type != null) { + Class paramClass = (Class) paramType.getActualTypeArguments()[0]; + if (paramClass != null) { + expectedClass(null, paramClass, msg + "::List"); + return; + } + } + } + + msg = msg + "<" + clazz.getName() + ">"; + if(tooHardToRecurseIntoField.contains(msg)) { + return; + } + + // How to fix: If you added a new type meant to be sent to the client, add it + // in one of the class List above. + Assert.fail("Unexpected type in View " + msg); + } +} From dd7f755c9b471af54c52b91e72967e49c5d0f7c4 Mon Sep 17 00:00:00 2001 From: Susucre <34709007+Susucre@users.noreply.github.com> Date: Tue, 19 Sep 2023 10:39:49 +0200 Subject: [PATCH 2/2] cleanup --- .../src/test/java/mage/view/ValidateViews.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Mage.Common/src/test/java/mage/view/ValidateViews.java b/Mage.Common/src/test/java/mage/view/ValidateViews.java index 0f992192a05..52ea1447eb0 100644 --- a/Mage.Common/src/test/java/mage/view/ValidateViews.java +++ b/Mage.Common/src/test/java/mage/view/ValidateViews.java @@ -1,10 +1,8 @@ package mage.view; -import org.checkerframework.checker.units.qual.C; import org.junit.Assert; import org.junit.Test; -import java.io.Serializable; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.lang.reflect.ParameterizedType; @@ -16,7 +14,7 @@ public class ValidateViews { // All those class are sent to client as Views or inside Views. // The test in this file is attempting to find any class that should // not be sent. - private static List> toValidate = Arrays.asList( + private static final List> toValidate = Arrays.asList( // Views AbilityPickerView.class, AbilityView.class, CardsView.class, CardView.class, ChatMessage.class, CombatGroupView.class, @@ -89,18 +87,18 @@ public class ValidateViews { ); // Those are safe java class we allow in Views. - private static List> safeClass = Arrays.asList( + private static final List> safeClass = Arrays.asList( boolean.class, int.class, long.class, float.class, String.class, Date.class, UUID.class ); // Those are non-public class - private static List nonPublicNames = Arrays.asList( + private static final List nonPublicNames = Arrays.asList( "mage.players.PlayableObjectRecord" ); // Those are excluded as Reflection on type of Map/Set/Collection is really hard. - private static List tooHardToRecurseInto = Arrays.asList( + private static final List tooHardToRecurseInto = Arrays.asList( "mage.view.CardsView", // "mage.view.CardsView", // "mage.view.CardsView", // @@ -118,7 +116,7 @@ public class ValidateViews { ); // Same, but for fields. - private static List tooHardToRecurseIntoField = Arrays.asList( + private static final List tooHardToRecurseIntoField = Arrays.asList( "mage.view.GameClientMessage: ::targets" // ); @@ -173,7 +171,7 @@ public class ValidateViews { } msg = msg + "<" + clazz.getName() + ">"; - if(tooHardToRecurseIntoField.contains(msg)) { + if (tooHardToRecurseIntoField.contains(msg)) { return; }