Extended test cases to verify that any CardRequest passed in works with no side effect.

New tests (and fixed updated CardRequest implementation) verify that any method used for Card Retrieval when receiving directly CardRequest formatted strings as cardName won't have any side effect. In particular, the code is reliable to be robust enough to support passing in both card name (only) or a full cardRequest string.

If the latter, any parameter in the request will be updated - accordingly - only when explicit params in function calls are provided.
Also, new tests demonstrate that the current implementation has NO side effect when testing for multiple combinations. Therefore, whatever is requested in requestString won't be overruled, unless really requested too.

forge-core/src/main/java/forge/card/CardDb.java

@@ -93,6 +93,9 @@ public final class CardDb implements ICardDatabase, IDeckGenPool {
         public static String compose(String cardName, String setCode) {
             setCode = setCode != null ? setCode : "";
             cardName = cardName != null ? cardName : "";
+            if (cardName.indexOf(NameSetSeparator) != -1)
+                // If cardName is another RequestString, just get card name and forget about the rest.
+                cardName = CardRequest.fromString(cardName).cardName;
             return cardName + NameSetSeparator + setCode;
         }
 
@@ -514,9 +517,9 @@ public final class CardDb implements ICardDatabase, IDeckGenPool {
                 return setFilter && artIndexFilter && collectorNumberFilter;
             }
         }));
-        if (candidates.isEmpty()) {
+        if (candidates.isEmpty())
             return null;
-        }
+
         PaperCard candidate = candidates.get(0);
         // Before returning make sure that actual candidate has Image.
         // If not, try to replace current candidate with one having image,
@@ -617,9 +620,10 @@ public final class CardDb implements ICardDatabase, IDeckGenPool {
         final CardRequest cr = CardRequest.fromString(cardInfo);
         // Check whether input `frame` is null. In that case, fallback to default SetPreference !-)
         final CardArtPreference artPref = artPreference != null ? artPreference : this.defaultCardArtPreference;
-        if (artIndex >= IPaperCard.DEFAULT_ART_INDEX && cr.artIndex < IPaperCard.DEFAULT_ART_INDEX) {
+        cr.artIndex = Math.max(cr.artIndex, IPaperCard.DEFAULT_ART_INDEX);
-            cr.artIndex = artIndex;
+        if (cr.artIndex != artIndex && artIndex > IPaperCard.DEFAULT_ART_INDEX )
-        }
+            cr.artIndex = artIndex;  // 2nd cond. is to verify that some actual value has been passed in.
+
         List<PaperCard> cards = getAllCards(cr.cardName);
         if (releaseDate != null) {
             cards = Lists.newArrayList(Iterables.filter(cards, new Predicate<PaperCard>() {
@@ -669,7 +673,7 @@ public final class CardDb implements ICardDatabase, IDeckGenPool {
             Collections.reverse(acceptedEditions);  // newest editions first
         PaperCard candidate = null;
         for (CardEdition ed : acceptedEditions) {
-            PaperCard cardFromSet = getCardFromSet(cr.cardName, ed, artIndex, cr.isFoil);
+            PaperCard cardFromSet = getCardFromSet(cr.cardName, ed, cr.artIndex, cr.isFoil);
             if (candidate == null && cardFromSet != null)
                 // save the first card found, as the last backup in case no other candidate *with image* will be found
                 candidate = cardFromSet;

forge-gui-desktop/src/test/java/forge/card/CardDbTestCase.java

@@ -1383,6 +1383,156 @@ public class CardDbTestCase extends ForgeCardMockTestCase {
         assertEquals(httCard.getEdition(), editionHymnToTourach);
         assertEquals(httCard.getCollectorNumber(), collectorNumbersHymnToTourach[0]);
         assertEquals(httCard.getArtIndex(), IPaperCard.DEFAULT_ART_INDEX);
+    }
+
+    @Test
+    public void testThatCardRequestPassedInHaveNoSideEffectAndThatAreCorrectlyProcessed(){
+        String cardName = this.cardNameHymnToTourach;
+        String httEdition = this.originalArtHymnToTourachEdition;
+        int artIndexFEM = 3;
+        String requestInfo = CardDb.CardRequest.compose(cardName, httEdition, artIndexFEM);
+
+        // assert default condition
+        assertEquals(this.cardDb.getCardArtPreference(), CardDb.CardArtPreference.LATEST_ART_ALL_EDITIONS);
+
+        // === Get Card
+        // == 1. Pass in Request Info
+        PaperCard hymnToTourachCard = this.cardDb.getCard(requestInfo);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        assertEquals(hymnToTourachCard.getEdition(), httEdition);
+        assertEquals(hymnToTourachCard.getArtIndex(), artIndexFEM);
+
+        // == 2. Pass in Card Name, Set Code, ArtIndex
+        hymnToTourachCard = this.cardDb.getCard(cardName, httEdition, artIndexFEM);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        assertEquals(hymnToTourachCard.getEdition(), httEdition);
+        assertEquals(hymnToTourachCard.getArtIndex(), artIndexFEM);
+
+        // == 3. Pass in RequestInfo as Card Name
+        hymnToTourachCard = this.cardDb.getCard(requestInfo, httEdition, artIndexFEM);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        assertEquals(hymnToTourachCard.getEdition(), httEdition);
+        assertEquals(hymnToTourachCard.getArtIndex(), artIndexFEM);
+
+        // == 4. Pass in RequestInfo as CardName but then requesting for different artIndex
+        hymnToTourachCard = this.cardDb.getCard(requestInfo, httEdition, 2);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        assertEquals(hymnToTourachCard.getEdition(), httEdition);
+        assertEquals(hymnToTourachCard.getArtIndex(), 2);
+
+        // == 5. Pass in RequestInfo as CardName but then requesting for different edition and artIndex
+        hymnToTourachCard = this.cardDb.getCard(requestInfo, latestArtHymnToTourachEdition, 1);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        assertEquals(hymnToTourachCard.getEdition(), latestArtHymnToTourachEdition);
+        assertEquals(hymnToTourachCard.getArtIndex(), 1);
+
+        // === Get Card From Set
+
+        // == 1. Reference with all expected params
+        hymnToTourachCard = this.cardDb.getCardFromSet(cardName, StaticData.instance().getCardEdition(httEdition), artIndexFEM, false);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        assertEquals(hymnToTourachCard.getEdition(), httEdition);
+        assertEquals(hymnToTourachCard.getArtIndex(), artIndexFEM);
+
+        // == 2. Pass in RequestInfo as Card Name
+        hymnToTourachCard = this.cardDb.getCardFromSet(requestInfo, StaticData.instance().getCardEdition(httEdition), artIndexFEM, false);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        assertEquals(hymnToTourachCard.getEdition(), httEdition);
+        assertEquals(hymnToTourachCard.getArtIndex(), artIndexFEM);
+
+        // == 3. Pass in RequestInfo but request for a different art Index
+        hymnToTourachCard = this.cardDb.getCardFromSet(requestInfo, StaticData.instance().getCardEdition(httEdition), 2, false);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        assertEquals(hymnToTourachCard.getEdition(), httEdition);
+        assertEquals(hymnToTourachCard.getArtIndex(), 2);
+
+        // == 4. Pass in RequestInfo as Card Name but request for a different art Index and Edition
+        hymnToTourachCard = this.cardDb.getCardFromSet(requestInfo,
+                StaticData.instance().getCardEdition(latestArtHymnToTourachEdition), 1, false);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        assertEquals(hymnToTourachCard.getEdition(), latestArtHymnToTourachEdition);
+        assertEquals(hymnToTourachCard.getArtIndex(), 1);
+
+        // === Get Card From Editions
+
+        // == 1. Reference case
+        hymnToTourachCard = this.cardDb.getCardFromEditions(cardName, CardDb.CardArtPreference.ORIGINAL_ART_ALL_EDITIONS);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        assertEquals(hymnToTourachCard.getEdition(), httEdition);
+        assertEquals(hymnToTourachCard.getArtIndex(), 1);
+
+        // == 2. Pass in Request String as CardName
+        hymnToTourachCard = this.cardDb.getCardFromEditions(requestInfo, CardDb.CardArtPreference.ORIGINAL_ART_ALL_EDITIONS);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        assertEquals(hymnToTourachCard.getEdition(), httEdition);
+        // expecting this because artIndex is already in Request Info
+        assertEquals(hymnToTourachCard.getArtIndex(), artIndexFEM);
+
+        // == 3. Changing CardArtPreference so that it would not be compliant with request
+        // STILL expecting to get in return whatever is in request as no extra param has been provided.
+        hymnToTourachCard = this.cardDb.getCardFromEditions(requestInfo, CardDb.CardArtPreference.LATEST_ART_ALL_EDITIONS);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        // expecting this edition as present in request info
+        assertEquals(hymnToTourachCard.getEdition(), httEdition);
+        // expecting this because artIndex is already in Request Info
+        assertEquals(hymnToTourachCard.getArtIndex(), 3);
+
+        // == 4. Changing Art Index (not default) so still requesting card via request String
+        hymnToTourachCard = this.cardDb.getCardFromEditions(requestInfo,
+                CardDb.CardArtPreference.LATEST_ART_ALL_EDITIONS, 2);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        // expecting this edition as present in request info
+        assertEquals(hymnToTourachCard.getEdition(), httEdition);
+        // artIndex should be overwritten this time, as it's provided and not default
+        assertEquals(hymnToTourachCard.getArtIndex(), 2);
+
+        // == 4. Changing Art Index (this time with default) = so initially requested artIndex won't get changed!
+        hymnToTourachCard = this.cardDb.getCardFromEditions(requestInfo,
+                CardDb.CardArtPreference.LATEST_ART_ALL_EDITIONS, 1);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        // expecting this edition as present in request info
+        assertEquals(hymnToTourachCard.getEdition(), httEdition);
+        // artIndex should still be the one requested in CardRequest as value passed is default
+        assertEquals(hymnToTourachCard.getArtIndex(), artIndexFEM);
+
+        // == 5. Passing in Card Name Only
+        hymnToTourachCard = this.cardDb.getCardFromEditions(cardName, CardDb.CardArtPreference.LATEST_ART_ALL_EDITIONS);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        // expecting this edition as returned due to CardArtPreference
+        assertEquals(hymnToTourachCard.getEdition(), latestArtHymnToTourachEdition);
+        // artIndex should be overwritten this time, as it's provided and not default
+        assertEquals(hymnToTourachCard.getArtIndex(), 1);
+
+        // == 6. Forcing in a specific Art Index will overrule Art Preference
+        hymnToTourachCard = this.cardDb.getCardFromEditions(cardName,
+                CardDb.CardArtPreference.LATEST_ART_ALL_EDITIONS, artIndexFEM);
+        assertNotNull(hymnToTourachCard);
+        assertEquals(hymnToTourachCard.getName(), cardName);
+        // expecting this edition as returned due to CardArtPreference
+        assertEquals(hymnToTourachCard.getEdition(), httEdition);
+        // artIndex should be overwritten this time, as it's provided and not default
+        assertEquals(hymnToTourachCard.getArtIndex(), artIndexFEM);
+
+
+
+
+
+
     }
 
     @Test

forge-gui-desktop/src/test/java/forge/card/CardRequestTestCase.java

@@ -159,7 +159,7 @@ public class CardRequestTestCase {
         assertEquals(request.collectorNumber, IPaperCard.NO_COLLECTOR_NUMBER);
 
         // foil
-        requestString = foilCardNameFoil + sep + foilEdition + sep + "[" + foilCollNr + "]";;
+        requestString = foilCardNameFoil + sep + foilEdition + sep + "[" + foilCollNr + "]";
         request = CardRequest.fromString(requestString);
         assertEquals(request.cardName, foilCardName);
         assertEquals(request.edition, foilEdition);
@@ -185,7 +185,7 @@ public class CardRequestTestCase {
         assertEquals(request.collectorNumber, IPaperCard.NO_COLLECTOR_NUMBER);
 
         // foil
-        requestString = foilCardNameFoil + sep + foilEdition + sep + 3 + sep +"[" + foilCollNr + "]";;
+        requestString = foilCardNameFoil + sep + foilEdition + sep + 3 + sep +"[" + foilCollNr + "]";
         request = CardRequest.fromString(requestString);
         assertEquals(request.cardName, foilCardName);
         assertEquals(request.edition, foilEdition);
@@ -194,4 +194,25 @@ public class CardRequestTestCase {
         assertEquals(request.collectorNumber, foilCollNr);
     }
 
+    @Test
+    public void testCreatingCardRequestUsingAnotherRequestStringAsCardName(){
+        String requestString = CardRequest.compose(cardName, edition, 1);
+        CardRequest request = CardRequest.fromString(requestString);
+
+        assertEquals(request.cardName, cardName);
+        assertEquals(request.edition, edition);
+        assertEquals(request.artIndex, 1);
+
+        String newRequestString = CardRequest.compose(requestString, foilEdition, 2);
+        CardRequest newRequest = CardRequest.fromString(newRequestString);
+
+        assertEquals(newRequest.cardName, cardName);
+        assertEquals(newRequest.edition, foilEdition);
+        assertEquals(newRequest.artIndex, 2);
+
+        assertEquals(request.cardName, newRequest.cardName);
+        assertNotEquals(request.edition, newRequest.edition);
+        assertNotEquals(request.artIndex, newRequest.artIndex);
+    }
+
 }