diff --git a/game-core/src/main/java/games/strategy/engine/data/ChangeAttachmentChange.java b/game-core/src/main/java/games/strategy/engine/data/ChangeAttachmentChange.java index c02ccd03007..4f5c108cade 100644 --- a/game-core/src/main/java/games/strategy/engine/data/ChangeAttachmentChange.java +++ b/game-core/src/main/java/games/strategy/engine/data/ChangeAttachmentChange.java @@ -1,6 +1,6 @@ package games.strategy.engine.data; -import static com.google.common.base.Preconditions.checkNotNull; +import java.util.Optional; /** A game data change that captures a change to an attachment property value. */ public class ChangeAttachmentChange extends Change { @@ -10,7 +10,7 @@ public class ChangeAttachmentChange extends Change { private final Object newValue; private final Object oldValue; private final String property; - private boolean clearFirst = false; + private final boolean clearFirst; /** * Initializes a new instance of the ChangeAttachmentChange class. @@ -21,13 +21,13 @@ public class ChangeAttachmentChange extends Change { */ public ChangeAttachmentChange( final IAttachment attachment, final Object newValue, final String property) { - checkNotNull(attachment, "null attachment; newValue: " + newValue + ", property: " + property); - - attachedTo = attachment.getAttachedTo(); - attachmentName = attachment.getName(); - oldValue = attachment.getPropertyOrThrow(property).getValue(); - this.newValue = newValue; - this.property = property; + this( + attachment.getAttachedTo(), + attachment.getName(), + newValue, + attachment.getPropertyOrThrow(property).getValue(), + property, + false); } /** @@ -38,15 +38,14 @@ public ChangeAttachmentChange( final IAttachment attachment, final Object newValue, final String property, - final boolean resetFirst) { - checkNotNull(attachment, "null attachment; newValue: " + newValue + ", property: " + property); - - attachedTo = attachment.getAttachedTo(); - clearFirst = resetFirst; - attachmentName = attachment.getName(); - oldValue = attachment.getPropertyOrThrow(property).getValue(); - this.newValue = newValue; - this.property = property; + final boolean clearFirst) { + this( + attachment.getAttachedTo(), + attachment.getName(), + newValue, + attachment.getPropertyOrThrow(property).getValue(), + property, + clearFirst); } /** @@ -59,13 +58,16 @@ public ChangeAttachmentChange( final Object newValue, final Object oldValue, final String property, - final boolean resetFirst) { - this.attachmentName = attachmentName; + final boolean clearFirst) { + this.attachmentName = + Optional.ofNullable(attachmentName) + .map(name -> name.replaceAll("ttatch", "ttach")) + .orElse(null); attachedTo = attachTo; this.newValue = newValue; this.oldValue = oldValue; this.property = property; - clearFirst = resetFirst; + this.clearFirst = clearFirst; } public Attachable getAttachedTo() { diff --git a/game-core/src/main/java/games/strategy/engine/data/DefaultAttachment.java b/game-core/src/main/java/games/strategy/engine/data/DefaultAttachment.java index d25d379d749..9de4b725672 100644 --- a/game-core/src/main/java/games/strategy/engine/data/DefaultAttachment.java +++ b/game-core/src/main/java/games/strategy/engine/data/DefaultAttachment.java @@ -115,7 +115,9 @@ public void setAttachedTo(final Attachable attachable) { @Override public String getName() { - return name; + return Optional.ofNullable(name) + .map(attachmentName -> attachmentName.replaceAll("ttatch", "ttach")) + .orElse(null); } @Override diff --git a/game-core/src/main/java/games/strategy/engine/data/NamedAttachable.java b/game-core/src/main/java/games/strategy/engine/data/NamedAttachable.java index 8ff0d1f6154..d0a779eebe6 100644 --- a/game-core/src/main/java/games/strategy/engine/data/NamedAttachable.java +++ b/game-core/src/main/java/games/strategy/engine/data/NamedAttachable.java @@ -26,7 +26,7 @@ public Map getAttachments() { @Override public void addAttachment(final String key, final IAttachment value) { - attachments.put(key, value); + attachments.put(key.replaceAll("ttatchment", "ttachment"), value); } @Override diff --git a/game-core/src/main/java/games/strategy/engine/data/gameparser/GameParser.java b/game-core/src/main/java/games/strategy/engine/data/gameparser/GameParser.java index aa3a35df94a..81920b71dbb 100644 --- a/game-core/src/main/java/games/strategy/engine/data/gameparser/GameParser.java +++ b/game-core/src/main/java/games/strategy/engine/data/gameparser/GameParser.java @@ -467,21 +467,21 @@ private void parseProperties(final PropertyList propertyList) { .orElseGet(() -> Strings.emptyToNull(current.getValue())); final boolean editable = current.isEditable(); - final String property = current.getName(); + final String propertyName = LegacyPropertyMapper.mapPropertyName(current.getName()); if (editable) { - parseEditableProperty(current, property, value); + parseEditableProperty(current, propertyName, value); } else { if (current.getBooleanProperty() != null) { - properties.set(property, Boolean.parseBoolean(value)); + properties.set(propertyName, Boolean.parseBoolean(value)); } else if (current.getStringProperty() != null) { - properties.set(property, value); + properties.set(propertyName, value); } else { try { final int integer = Integer.parseInt(value); - properties.set(property, integer); + properties.set(propertyName, integer); } catch (final NumberFormatException e) { // then it must be a string - properties.set(property, value); + properties.set(propertyName, value); } } } @@ -900,7 +900,11 @@ private List> setOptions( final List> results = new ArrayList<>(); for (final AttachmentList.Attachment.Option option : options) { // decapitalize the property name for backwards compatibility - final String name = decapitalize(option.getName()); + final String name = LegacyPropertyMapper.mapLegacyOptionName(decapitalize(option.getName())); + if (LegacyPropertyMapper.ignoreOptionName(name)) { + continue; + } + if (name.isEmpty()) { throw new GameParseException( "Option name with zero length for attachment: " + attachment.getName()); @@ -912,7 +916,8 @@ private List> setOptions( continue; // Skip adding option if contains empty foreach variable } final String valueWithForeach = replaceForeachVariables(countAndValue, foreach); - final String finalValue = replaceVariables(valueWithForeach, variables); + final String interpolatedValue = replaceVariables(valueWithForeach, variables); + final String finalValue = LegacyPropertyMapper.mapLegacyOptionValue(name, interpolatedValue); try { attachment .getProperty(name) @@ -928,8 +933,8 @@ private List> setOptions( } catch (final Exception e) { throw new GameParseException( String.format( - "map name: '%s', Unexpected Exception while setting values for attachment: %s", - mapName, attachment), + "map name: '%s', Unexpected Exception while setting values for attachment: %s, %s", + mapName, attachment, e.getMessage()), e); } results.add(Tuple.of(name, finalValue)); diff --git a/game-core/src/main/java/games/strategy/engine/data/gameparser/LegacyPropertyMapper.java b/game-core/src/main/java/games/strategy/engine/data/gameparser/LegacyPropertyMapper.java new file mode 100644 index 00000000000..1c8ab9eb59b --- /dev/null +++ b/game-core/src/main/java/games/strategy/engine/data/gameparser/LegacyPropertyMapper.java @@ -0,0 +1,58 @@ +package games.strategy.engine.data.gameparser; + +import games.strategy.triplea.Constants; +import lombok.experimental.UtilityClass; + +/** + * This converts property names and values that were used in older map XMLs and updates those values + * to their newer versions. This is useful to allow the game code to only use the newer names and + * values while still being able to load older maps. + */ +@UtilityClass +class LegacyPropertyMapper { + + static String mapLegacyOptionName(final String optionName) { + if (optionName.equalsIgnoreCase("isParatroop")) { + return "isAirTransportable"; + } else if (optionName.equalsIgnoreCase("isInfantry") + || optionName.equalsIgnoreCase("isMechanized")) { + return "isLandTransportable"; + } else if (optionName.equalsIgnoreCase("occupiedTerrOf")) { + return Constants.ORIGINAL_OWNER; + } else if (optionName.equalsIgnoreCase("isImpassible")) { + return "isImpassable"; + } else if (optionName.equalsIgnoreCase("turns")) { + return "rounds"; + } + return optionName; + } + + public String mapLegacyOptionValue(final String optionName, final String optionValue) { + if (optionName.equalsIgnoreCase("victoryCity")) { + if (optionValue.equalsIgnoreCase("true")) { + return "1"; + } else if (optionValue.equalsIgnoreCase("false")) { + return "0"; + } else { + return optionValue; + } + } + return optionValue; + } + + public String mapPropertyName(final String propertyName) { + if (propertyName.equalsIgnoreCase("Battleships repair at end of round") + || propertyName.equalsIgnoreCase("Units repair at end of round")) { + return Constants.TWO_HIT_BATTLESHIPS_REPAIR_END_OF_TURN; + } else if (propertyName.equalsIgnoreCase("Battleships repair at beginning of round") + || propertyName.equalsIgnoreCase("Units repair at beginning of round")) { + return Constants.TWO_HIT_BATTLESHIPS_REPAIR_BEGINNING_OF_TURN; + } + + return propertyName; + } + + static boolean ignoreOptionName(final String name) { + return name.equalsIgnoreCase("takeUnitControl") || name.equalsIgnoreCase("giveUnitControl"); + } +} diff --git a/game-core/src/main/java/games/strategy/engine/data/gameparser/XmlGameElementMapper.java b/game-core/src/main/java/games/strategy/engine/data/gameparser/XmlGameElementMapper.java index 587c7876e5d..db45be02a52 100644 --- a/game-core/src/main/java/games/strategy/engine/data/gameparser/XmlGameElementMapper.java +++ b/game-core/src/main/java/games/strategy/engine/data/gameparser/XmlGameElementMapper.java @@ -175,8 +175,7 @@ public Optional newAttachment( final GameData gameData) { checkNotNull(typeName); - final String normalizedTypeName = - typeName.replaceAll("^games\\.strategy\\.triplea\\.attachments\\.", ""); + final String normalizedTypeName = typeName.replaceAll("^.*\\.", ""); final @Nullable AttachmentFactory attachmentFactory = attachmentFactoriesByTypeName.get(normalizedTypeName); if (attachmentFactory != null) { diff --git a/game-core/src/main/java/games/strategy/triplea/attachments/AbstractRulesAttachment.java b/game-core/src/main/java/games/strategy/triplea/attachments/AbstractRulesAttachment.java index 9b2f65cf7b0..73b370286ac 100644 --- a/game-core/src/main/java/games/strategy/triplea/attachments/AbstractRulesAttachment.java +++ b/game-core/src/main/java/games/strategy/triplea/attachments/AbstractRulesAttachment.java @@ -1,5 +1,6 @@ package games.strategy.triplea.attachments; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; import games.strategy.engine.data.Attachable; import games.strategy.engine.data.GameData; @@ -18,6 +19,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import org.triplea.java.RenameOnNextMajorRelease; import org.triplea.java.collections.CollectionUtils; /** The Purpose of this class is to hold shared and simple methods used by RulesAttachment. */ @@ -34,6 +36,7 @@ public abstract class AbstractRulesAttachment extends AbstractConditionsAttachme // only matters for objectiveValue, does not affect the condition protected int uses = -1; // condition for what turn it is + @RenameOnNextMajorRelease(newName = "rounds") protected Map turns = null; // for on/off conditions protected boolean switched = true; @@ -206,19 +209,16 @@ private void setRounds(final String rounds) throws GameParseException { } } - private void setTurns(final String turns) throws GameParseException { - setRounds(turns); - } - - private void setTurns(final Map value) { + private void setRounds(final Map value) { turns = value; } - private Map getTurns() { + @VisibleForTesting + public Map getRounds() { return turns; } - private void resetTurns() { + private void resetRounds() { turns = null; } @@ -433,8 +433,9 @@ public Map> getPropertyMap() { "uses", MutableProperty.of(this::setUses, this::setUses, this::getUses, this::resetUses)) .put( - "turns", - MutableProperty.of(this::setTurns, this::setTurns, this::getTurns, this::resetTurns)) + "rounds", + MutableProperty.of( + this::setRounds, this::setRounds, this::getRounds, this::resetRounds)) .put( "switch", MutableProperty.of( @@ -443,7 +444,6 @@ public Map> getPropertyMap() { "gameProperty", MutableProperty.ofString( this::setGameProperty, this::getGameProperty, this::resetGameProperty)) - .put("rounds", MutableProperty.ofWriteOnlyString(this::setRounds)) .build(); } } diff --git a/game-core/src/main/java/games/strategy/triplea/attachments/TriggerAttachment.java b/game-core/src/main/java/games/strategy/triplea/attachments/TriggerAttachment.java index 8af41367871..ebdde06a400 100644 --- a/game-core/src/main/java/games/strategy/triplea/attachments/TriggerAttachment.java +++ b/game-core/src/main/java/games/strategy/triplea/attachments/TriggerAttachment.java @@ -908,11 +908,13 @@ private void resetPlayers() { players = new ArrayList<>(); } - private void setPlayerAttachmentName(final String name) throws GameParseException { - if (name == null) { - playerAttachmentName = null; + private void setPlayerAttachmentName(final String playerAttachmentName) + throws GameParseException { + if (playerAttachmentName == null) { + this.playerAttachmentName = null; return; } + final String name = playerAttachmentName.replaceAll("ttatch", "ttach"); final String[] s = splitOnColon(name); if (s.length != 2) { throw new GameParseException( @@ -961,7 +963,7 @@ private void setPlayerAttachmentName(final String name) throws GameParseExceptio && !s[0].startsWith(Constants.USERACTION_ATTACHMENT_PREFIX)) { throw new GameParseException("attachment incorrectly named:" + s[0] + thisErrorMsg()); } - playerAttachmentName = Tuple.of(s[1], s[0]); + this.playerAttachmentName = Tuple.of(s[1], s[0]); } private void setPlayerAttachmentName(final Tuple value) { diff --git a/game-core/src/main/java/games/strategy/triplea/attachments/UnitAttachment.java b/game-core/src/main/java/games/strategy/triplea/attachments/UnitAttachment.java index 812ba90e59a..e4d21f059cd 100644 --- a/game-core/src/main/java/games/strategy/triplea/attachments/UnitAttachment.java +++ b/game-core/src/main/java/games/strategy/triplea/attachments/UnitAttachment.java @@ -3690,12 +3690,6 @@ public Map> getPropertyMap() { .put( "isAir", MutableProperty.of(this::setIsAir, this::setIsAir, this::getIsAir, this::resetIsAir)) - .put( - "isMechanized", // kept for map compatibility; remove upon next map-incompatible release - MutableProperty.ofWriteOnlyString(s -> {})) - .put( - "isParatroop", // kept for map compatibility; remove upon next map-incompatible release - MutableProperty.ofWriteOnlyString(s -> {})) .put( "isSea", MutableProperty.of(this::setIsSea, this::setIsSea, this::getIsSea, this::resetIsSea)) @@ -3943,13 +3937,6 @@ public Map> getPropertyMap() { this::setIsAirTransportable, this::getIsAirTransportable, this::resetIsAirTransportable)) - .put( - "isInfantry", // kept for map compatibility; remove upon next map-incompatible release - MutableProperty.of( - this::setIsLandTransportable, - this::setIsLandTransportable, - this::getIsLandTransportable, - this::resetIsLandTransportable)) .put( "isLandTransport", MutableProperty.of( diff --git a/game-core/src/main/java/games/strategy/triplea/ui/mapdata/MapData.java b/game-core/src/main/java/games/strategy/triplea/ui/mapdata/MapData.java index 038a355ccca..51fd534fa86 100644 --- a/game-core/src/main/java/games/strategy/triplea/ui/mapdata/MapData.java +++ b/game-core/src/main/java/games/strategy/triplea/ui/mapdata/MapData.java @@ -500,8 +500,10 @@ public Color getPlayerColor(final String playerName) { /** returns the color for impassable territories. */ public Color impassableColor() { - // just use getPlayerColor, since it parses the properties - return getPlayerColor(Constants.PLAYER_NAME_IMPASSABLE); + return Optional.ofNullable( + getColorProperty(PROPERTY_COLOR_PREFIX + Constants.PLAYER_NAME_IMPASSABLE)) + .or(() -> Optional.ofNullable(getColorProperty(PROPERTY_COLOR_PREFIX + "Impassible"))) + .orElseGet(defaultColors::nextColor); } /** diff --git a/game-core/src/test/java/games/strategy/engine/data/gameparser/GameParserTest.java b/game-core/src/test/java/games/strategy/engine/data/gameparser/GameParserTest.java index a57081092d5..bb80f8c1ff6 100644 --- a/game-core/src/test/java/games/strategy/engine/data/gameparser/GameParserTest.java +++ b/game-core/src/test/java/games/strategy/engine/data/gameparser/GameParserTest.java @@ -1,14 +1,136 @@ package games.strategy.engine.data.gameparser; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; +import static org.hamcrest.core.Is.is; +import static org.hamcrest.core.IsNull.notNullValue; +import games.strategy.engine.data.GameData; +import games.strategy.triplea.Constants; +import games.strategy.triplea.attachments.RulesAttachment; +import games.strategy.triplea.attachments.TerritoryAttachment; +import games.strategy.triplea.attachments.UnitAttachment; +import java.net.URI; import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.triplea.util.Tuple; final class GameParserTest { + + @Test + @DisplayName("Verify backward compatibility can parse 1.8 maps") + void backwardCompatibilityCheck() throws Exception { + final URI mapUri = + GameParserTest.class.getClassLoader().getResource("v1_8_map__270BC.xml").toURI(); + + final GameData gameData = GameParser.parse(mapUri, new XmlGameElementMapper()).orElseThrow(); + assertNotNullGameData(gameData); + + verifyLegacyPropertiesAreUpdated(gameData); + } + + /** Asserts that we loaded a relatively complete looking game data. */ + private void assertNotNullGameData(final GameData gameData) { + assertThat(gameData.getAttachmentOrderAndValues(), is(notNullValue())); + assertThat(gameData.getAllianceTracker().getAlliances(), is(notNullValue())); + assertThat(gameData.getBattleRecordsList(), is(notNullValue())); + assertThat(gameData.getDelegates(), is(notNullValue())); + assertThat(gameData.getDiceSides(), is(notNullValue())); + assertThat(gameData.getGameLoader(), is(notNullValue())); + assertThat(gameData.getGameName(), is(notNullValue())); + assertThat(gameData.getGameVersion(), is(notNullValue())); + assertThat(gameData.getHistory().getActivePlayer(), is(notNullValue())); + assertThat(gameData.getHistory().getLastNode(), is(notNullValue())); + assertThat(gameData.getMap().getTerritories(), is(notNullValue())); + assertThat(gameData.getPlayerList().getPlayers(), is(notNullValue())); + assertThat( + gameData.getProductionFrontierList().getProductionFrontierNames(), is(notNullValue())); + assertThat(gameData.getProductionRuleList().getProductionRules(), is(notNullValue())); + assertThat(gameData.getProperties(), is(notNullValue())); + assertThat(gameData.getRelationshipTracker(), is(notNullValue())); + assertThat(gameData.getRelationshipTypeList().getAllRelationshipTypes(), is(notNullValue())); + assertThat(gameData.getRepairFrontierList().getRepairFrontierNames(), is(notNullValue())); + assertThat(gameData.getResourceList().getResources(), is(notNullValue())); + assertThat(gameData.getSaveGameFileName(), is(notNullValue())); + assertThat(gameData.getSequence().getRound(), is(notNullValue())); + assertThat(gameData.getSequence().getStep(), is(notNullValue())); + assertThat(gameData.getTechnologyFrontier().getTechs(), is(notNullValue())); + assertThat(gameData.getTerritoryEffectList(), is(notNullValue())); + assertThat(gameData.getUnits().getUnits(), is(notNullValue())); + assertThat(gameData.getUnitTypeList().getAllUnitTypes(), is(notNullValue())); + } + + /** + * The test-XML is intentinally loaded with legacy properties and options. Here we assert that + * those legacy values have been forward-ported to their new, non-legacy values. + */ + private void verifyLegacyPropertiesAreUpdated(final GameData gameData) { + assertThat( + gameData.getProperties().get(Constants.TWO_HIT_BATTLESHIPS_REPAIR_END_OF_TURN), is(true)); + assertThat( + gameData.getProperties().get(Constants.TWO_HIT_BATTLESHIPS_REPAIR_BEGINNING_OF_TURN), + is(true)); + + final var spartaTerritoryAttachment = + (TerritoryAttachment) + gameData + .getMap() + .getTerritory("Sparta") + .getAttachment(Constants.TERRITORY_ATTACHMENT_NAME); + + assertThat(spartaTerritoryAttachment.getVictoryCity(), is(1)); + assertThat(spartaTerritoryAttachment.getOriginalOwner().getName(), is("RomanRepublic")); + assertThat(spartaTerritoryAttachment.getIsImpassable(), is(true)); + + final var romaTerritoryAttachment = + (TerritoryAttachment) + gameData + .getMap() + .getTerritory("Roma") + .getAttachment(Constants.TERRITORY_ATTACHMENT_NAME); + + assertThat(romaTerritoryAttachment.getVictoryCity(), is(0)); + + final var archerUnitAttachment = + (UnitAttachment) + gameData + .getUnitTypeList() + .getUnitType("archer") + .getAttachment(Constants.UNIT_ATTACHMENT_NAME); + + assertThat( + "Verify isTwoHitPoint=true is converted to hitPoints = 2", + archerUnitAttachment.getHitPoints(), + is(2)); + assertThat( + "Verify is paratroop is converted", archerUnitAttachment.getIsAirTransportable(), is(true)); + assertThat( + "Verify isMechanized is converted", + archerUnitAttachment.getIsLandTransportable(), + is(true)); + + final var axemanUnitAttachment = + ((UnitAttachment) + gameData + .getUnitTypeList() + .getUnitType("axeman") + .getAttachment(Constants.UNIT_ATTACHMENT_NAME)); + + assertThat( + "Verify isInfantry is converted", axemanUnitAttachment.getIsLandTransportable(), is(true)); + + assertThat( + ((RulesAttachment) + gameData + .getPlayerList() + .getPlayerId("Carthage") + .getAttachment("conditionAttachmentAntiRomanVictory8")) + .getRounds(), + is(Map.of(1, 1, 2, 2))); + } + @Nested final class DecapitalizeTest { @Test diff --git a/game-core/src/test/resources/v1_8_map__270BC.xml b/game-core/src/test/resources/v1_8_map__270BC.xml new file mode 100644 index 00000000000..35d983c361f --- /dev/null +++ b/game-core/src/test/resources/v1_8_map__270BC.xml @@ -0,0 +1,2941 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/java-extras/src/main/java/org/triplea/java/RenameOnNextMajorRelease.java b/java-extras/src/main/java/org/triplea/java/RenameOnNextMajorRelease.java new file mode 100644 index 00000000000..0c9120780c3 --- /dev/null +++ b/java-extras/src/main/java/org/triplea/java/RenameOnNextMajorRelease.java @@ -0,0 +1,13 @@ +package org.triplea.java; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +/** + * Annotation indicating an element should be renamed on the next major release when we are okay + * breaking save game compatibility. + */ +@Retention(RetentionPolicy.SOURCE) +public @interface RenameOnNextMajorRelease { + String newName(); +} diff --git a/map-data/src/main/java/org/triplea/map/data/elements/AttachmentList.java b/map-data/src/main/java/org/triplea/map/data/elements/AttachmentList.java index 4516a6d3949..de8088b139d 100644 --- a/map-data/src/main/java/org/triplea/map/data/elements/AttachmentList.java +++ b/map-data/src/main/java/org/triplea/map/data/elements/AttachmentList.java @@ -7,13 +7,17 @@ @Getter public class AttachmentList { - @TagList private List attachments; + @TagList(names = {"Attachment", "Attatchment"}) + private List attachments; @Getter public static class Attachment { @Attribute private String foreach; @Attribute private String name; - @Attribute private String attachTo; + + @Attribute(names = {"attachTo", "attatchTo"}) + private String attachTo; + @Attribute private String javaClass; @Attribute(defaultValue = "unitType") diff --git a/map-data/src/main/java/org/triplea/map/data/elements/Game.java b/map-data/src/main/java/org/triplea/map/data/elements/Game.java index e551ced512e..5d814493ca9 100644 --- a/map-data/src/main/java/org/triplea/map/data/elements/Game.java +++ b/map-data/src/main/java/org/triplea/map/data/elements/Game.java @@ -12,7 +12,10 @@ public class Game { @Tag private Info info; @Tag private Triplea triplea; - @Tag private AttachmentList attachmentList; + + @Tag(names = {"attachmentList", "attatchmentList"}) + private AttachmentList attachmentList; + @Tag private DiceSides diceSides; @Tag private GamePlay gamePlay; @Tag private Initialize initialize; diff --git a/xml-reader/src/main/java/org/triplea/generic/xml/reader/XmlMapper.java b/xml-reader/src/main/java/org/triplea/generic/xml/reader/XmlMapper.java index 7ea9855466c..8ad8c865421 100644 --- a/xml-reader/src/main/java/org/triplea/generic/xml/reader/XmlMapper.java +++ b/xml-reader/src/main/java/org/triplea/generic/xml/reader/XmlMapper.java @@ -5,12 +5,17 @@ import java.io.InputStream; import java.lang.reflect.Field; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import java.util.Objects; import java.util.logging.Level; import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; import lombok.extern.java.Log; +import org.triplea.generic.xml.reader.annotations.Attribute; +import org.triplea.generic.xml.reader.annotations.Tag; +import org.triplea.generic.xml.reader.annotations.TagList; import org.triplea.generic.xml.reader.exceptions.JavaDataModelException; import org.triplea.generic.xml.reader.exceptions.XmlParsingException; @@ -28,6 +33,11 @@ public XmlMapper(final InputStream inputStream) throws XmlParsingException { } public T mapXmlToObject(final Class pojo) throws XmlParsingException { + return mapXmlToObject(pojo, pojo.getSimpleName()); + } + + public T mapXmlToObject(final Class pojo, final String tagName) + throws XmlParsingException { // At this point in parsing the XML cursor is just beyond the start tag. // We can read attributes directly off of the stream at this point. // If we do nothing more then the cursor will keep moving down and will not @@ -60,8 +70,19 @@ public T mapXmlToObject(final Class pojo) throws XmlParsingException { // set attributes on the current object for (final Field field : annotatedFields.getAttributeFields()) { - final String xmlAttributeValue = xmlStreamReader.getAttributeValue(null, field.getName()); - final Object value = new AttributeValueCasting(field).castAttributeValue(xmlAttributeValue); + + final String[] attributeNames = + getNamesFromAnnotationOrDefault( + field.getAnnotation(Attribute.class).names(), field.getName()); + + final String attributeValue = + Arrays.stream(attributeNames) + .map(attributeName -> xmlStreamReader.getAttributeValue(null, attributeName)) + .filter(Objects::nonNull) + .findAny() + .orElse(null); + + final Object value = new AttributeValueCasting(field).castAttributeValue(attributeValue); field.set(instance, value); } @@ -74,16 +95,25 @@ public T mapXmlToObject(final Class pojo) throws XmlParsingException { // This parser will do the work of parsing the current tag, it'll look at all // child tags and the body text and invoke the right callback that we will define below. - final XmlParser tagParser = new XmlParser(pojo.getSimpleName()); + final XmlParser tagParser = new XmlParser(tagName); // Set up tag parsing, as we scan through more elements when we see a matching // tag name we'll call the child tag handler. The child tag handler will // create a java model representing the child tag and set the field instance // on our current running instance object. for (final Field field : annotatedFields.getTagFields()) { - final String expectedTagName = field.getType().getSimpleName(); - tagParser.childTagHandler( - expectedTagName, () -> field.set(instance, mapXmlToObject(field.getType()))); + + final String[] tagNames = + getNamesFromAnnotationOrDefault( + field.getAnnotation(Tag.class).names(), field.getType().getSimpleName()); + + Arrays.stream(tagNames) + .forEach( + expectedTagName -> + tagParser.childTagHandler( + expectedTagName, + () -> + field.set(instance, mapXmlToObject(field.getType(), expectedTagName)))); } // Set up tag list parsing, similar to tag parsing except we set the field @@ -92,8 +122,17 @@ public T mapXmlToObject(final Class pojo) throws XmlParsingException { final List tagList = new ArrayList<>(); field.set(instance, tagList); final Class listType = ReflectionUtils.getGenericType(field); - tagParser.childTagHandler( - listType.getSimpleName(), () -> tagList.add(mapXmlToObject(listType))); + + final String[] tagNames = + getNamesFromAnnotationOrDefault( + field.getAnnotation(TagList.class).names(), listType.getSimpleName()); + + Arrays.stream(tagNames) + .forEach( + expectedTagName -> + tagParser.childTagHandler( + expectedTagName, + () -> tagList.add(mapXmlToObject(listType, expectedTagName)))); } // Set up body text handler. The XML cursor will iterate over each line of body @@ -123,6 +162,13 @@ public T mapXmlToObject(final Class pojo) throws XmlParsingException { } } + private static String[] getNamesFromAnnotationOrDefault( + final String[] annotationValues, final String defaultValue) { + return annotationValues.length == 1 && annotationValues[0].isEmpty() + ? new String[] {defaultValue} + : annotationValues; + } + @Override public void close() { try { diff --git a/xml-reader/src/main/java/org/triplea/generic/xml/reader/annotations/Attribute.java b/xml-reader/src/main/java/org/triplea/generic/xml/reader/annotations/Attribute.java index 646b0cf924c..c0bb783561c 100644 --- a/xml-reader/src/main/java/org/triplea/generic/xml/reader/annotations/Attribute.java +++ b/xml-reader/src/main/java/org/triplea/generic/xml/reader/annotations/Attribute.java @@ -29,6 +29,20 @@ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.FIELD) public @interface Attribute { + /** + * By default the searched for attribute name will be be the name of annotated java field. This + * property overrides that default to search for a different set of names.For example, the below + * annotation would match attributes "foo" and "bar", but not "attribute". + * + * 
{@code
+   * class Tag {
+   *   @Attribute(names = {"foo" , "bar"}
+   *   private String attribute;
+   * }
+   * }
+ */ + String[] names() default ""; + String defaultValue() default ""; int defaultInt() default 0; diff --git a/xml-reader/src/main/java/org/triplea/generic/xml/reader/annotations/Tag.java b/xml-reader/src/main/java/org/triplea/generic/xml/reader/annotations/Tag.java index 4300a3a30a9..adca7695d89 100644 --- a/xml-reader/src/main/java/org/triplea/generic/xml/reader/annotations/Tag.java +++ b/xml-reader/src/main/java/org/triplea/generic/xml/reader/annotations/Tag.java @@ -7,4 +7,18 @@ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.FIELD) -public @interface Tag {} +public @interface Tag { + /** + * By default the searched for tag name will be be the name of annotated java object type. This + * property overrides that default to search for a different set of names.For example, the below + * annotation would match tags named "foo" and "bar", but not "tag" . + * + * 
{@code
+   * class Tag {
+   *   @Tag(names = {"foo" , "bar"}
+   *   private Tag tagObjectName;
+   * }
+   * }
+ */ + String[] names() default ""; +} diff --git a/xml-reader/src/main/java/org/triplea/generic/xml/reader/annotations/TagList.java b/xml-reader/src/main/java/org/triplea/generic/xml/reader/annotations/TagList.java index b44679df3e0..2050349fa69 100644 --- a/xml-reader/src/main/java/org/triplea/generic/xml/reader/annotations/TagList.java +++ b/xml-reader/src/main/java/org/triplea/generic/xml/reader/annotations/TagList.java @@ -7,4 +7,24 @@ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.FIELD) -public @interface TagList {} +public @interface TagList { + /** + * By default the searched for tag name will be be the name of annotated java object list generic + * type. This property overrides that default to search for a different set of names.For example, + * the below annotation would match tags named "foo" and "bar", but not "tag" . + * + * 
{@code
+   * class Tag {
+   *   @TagList(names = {"foo" , "bar"}
+   *   private List tagList;
+   * }
+   *
+   * // matches
+   * 
+   *       
+   *       
+   *   
+   * }
+ */ + String[] names() default ""; +} diff --git a/xml-reader/src/test/java/org/triplea/generic/xml/reader/TagAlternativeSpellingTest.java b/xml-reader/src/test/java/org/triplea/generic/xml/reader/TagAlternativeSpellingTest.java new file mode 100644 index 00000000000..3f23e1d5298 --- /dev/null +++ b/xml-reader/src/test/java/org/triplea/generic/xml/reader/TagAlternativeSpellingTest.java @@ -0,0 +1,67 @@ +package org.triplea.generic.xml.reader; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.collection.IsCollectionWithSize.hasSize; +import static org.hamcrest.core.Is.is; +import static org.hamcrest.core.IsNull.notNullValue; + +import java.util.List; +import lombok.Getter; +import org.junit.jupiter.api.Test; +import org.triplea.generic.xml.reader.annotations.Attribute; +import org.triplea.generic.xml.reader.annotations.Tag; +import org.triplea.generic.xml.reader.annotations.TagList; + +/** + * Verifies that we can give tags, taglists and attributes alternative names and correctly match + * XMLs that contain those alternative names. + */ +@SuppressWarnings("UnmatchedTest") +public class TagAlternativeSpellingTest extends AbstractXmlMapperTest { + + TagAlternativeSpellingTest() { + super("library-example-for-alt-spellings.xml"); + } + + /** POJO modelling the XML in our sample dataset. */ + @Getter + public static class Library { + @Tag(names = {"inventory"}) + private Catalog catalog; + + @Getter + public static class Catalog { + @TagList(names = {"items"}) + private List libraryItems; + + public static class LibraryItem { + @TagList(names = {"Book", "DVD"}) + private List
articles; + + @Getter + public static class Article { + @Attribute(names = {"name"}) + private String title; + } + } + } + } + + @Test + void verifySimpleExample() throws Exception { + final Library library = xmlMapper.mapXmlToObject(Library.class); + + assertThat(library, is(notNullValue())); + assertThat(library.catalog, is(notNullValue())); + assertThat(library.catalog.libraryItems, hasSize(2)); + assertThat(library.catalog.libraryItems.get(0).articles, hasSize(2)); + assertThat( + library.catalog.libraryItems.get(0).articles.get(0).title, is("Crossing the Atlantic")); + assertThat( + library.catalog.libraryItems.get(0).articles.get(1).title, is("The Battle of the Bulge")); + + assertThat(library.catalog.libraryItems.get(1).articles, hasSize(2)); + assertThat(library.catalog.libraryItems.get(1).articles.get(0).title, is("How to Win Revised")); + assertThat(library.catalog.libraryItems.get(1).articles.get(1).title, is("Game of TripleA")); + } +} diff --git a/xml-reader/src/test/resources/library-example-for-alt-spellings.xml b/xml-reader/src/test/resources/library-example-for-alt-spellings.xml new file mode 100644 index 00000000000..2c3310b2046 --- /dev/null +++ b/xml-reader/src/test/resources/library-example-for-alt-spellings.xml @@ -0,0 +1,12 @@ + + + + + + + + + + + + diff --git a/xml-reader/src/test/resources/simple-library-example.xml b/xml-reader/src/test/resources/simple-library-example.xml index ad119200d28..6b1faa47d2b 100644 --- a/xml-reader/src/test/resources/simple-library-example.xml +++ b/xml-reader/src/test/resources/simple-library-example.xml @@ -5,6 +5,8 @@ - +