Merge branch 'patch-imagefetcher-scryfall' into 'master'

FIX ImageFetcher against Scryfall, so that download can be robust (against different set-codes) and reliable.

See merge request core-developers/forge!4788

forge-core/src/main/java/forge/util/ImageUtil.java

@@ -99,7 +99,7 @@ public class ImageUtil {
                     final CardDb db =  StaticData.instance().getCommonCards();
                     return db.getRules(card.getMeldWith()).getOtherPart().getName();
                 } else {
-            	    return null;
+                    return null;
                 }
             else
                 return null;
@@ -118,6 +118,27 @@ public class ImageUtil {
         return getImageRelativePath(cp, backFace, true, true);
     }
 
+    public static String getScryfallDownloadUrl(PaperCard cp, boolean backFace, String setCode){
+        return getScryfallDownloadUrl(cp, backFace, setCode, "en");
+    }
+
+    public static String getScryfallDownloadUrl(PaperCard cp, boolean backFace, String setCode, String langCode){
+        String editionCode;
+        if ((setCode != null) && (setCode.length() > 0))
+            editionCode = setCode;
+        else
+            editionCode = cp.getEdition().toLowerCase();
+        String cardCollectorNumber = cp.getCollectorNumber();
+        // Hack to account for variations in Arabian Nights
+        cardCollectorNumber = cardCollectorNumber.replace("+", "†");
+        String faceParam = "";
+        if (cp.getRules().getOtherPart() != null) {
+            faceParam = (backFace ? "&face=back" : "&face=front");
+        }
+        return String.format("%s/%s/%s?format=image&version=normal%s", editionCode, cardCollectorNumber,
+                langCode, faceParam);
+    }
+
     public static String toMWSFilename(String in) {
         final StringBuilder out = new StringBuilder();
         char c;
@@ -130,4 +151,4 @@ public class ImageUtil {
         }
         return out.toString();
     }
-}
+}

forge-gui/src/main/java/forge/util/ImageFetcher.java

@@ -1,14 +1,14 @@
 package forge.util;
 
 import java.io.File;
-import java.util.ArrayList;
+import java.io.IOException;
-import java.util.HashMap;
+import java.net.HttpURLConnection;
-import java.util.HashSet;
+import java.net.URL;
+import java.util.*;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
+import forge.card.CardEdition;
 import forge.item.IPaperCard;
 import org.apache.commons.lang3.tuple.Pair;
 
@@ -25,6 +25,9 @@ public abstract class ImageFetcher {
     // see https://scryfall.com/docs/api/languages and
     // https://en.wikipedia.org/wiki/List_of_ISO_639-1_codes
     private static final HashMap<String, String> langCodeMap = new HashMap<>();
+    private static final Map<String, String> scryfallSetCodes = new HashMap<>();
+    private static final String INVALID_SCRYFALL_SET_CODE = "NotFound";
+
     static {
         langCodeMap.put("en-US", "en");
         langCodeMap.put("es-ES", "es");
@@ -41,6 +44,65 @@ public abstract class ImageFetcher {
     private HashMap<String, HashSet<Callback>> currentFetches = new HashMap<>();
     private HashMap<String, String> tokenImages;
 
+    private static boolean isValidScryfallURL(final String urlString){
+        try {
+            URL u = new URL(urlString);
+            HttpURLConnection huc = (HttpURLConnection) u.openConnection();
+            huc.setInstanceFollowRedirects(true);
+            huc.setRequestProperty("User-Agent", "Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.2) " +
+                    "Gecko/20090729 Firefox/3.5.2 (.NET CLR 3.5.30729)");
+            huc.setRequestMethod("HEAD");
+            return (huc.getResponseCode() == HttpURLConnection.HTTP_OK);
+        } catch (IOException e) {
+            return false;
+        }
+
+    }
+
+    private String getScryfallDownloadURL(PaperCard c, boolean backFace, String langCode){
+        String setCode = scryfallSetCodes.getOrDefault(c.getEdition(), null);
+        if ((setCode != null) && (!setCode.equals(INVALID_SCRYFALL_SET_CODE))){
+                return ForgeConstants.URL_PIC_SCRYFALL_DOWNLOAD +
+                        ImageUtil.getScryfallDownloadUrl(c, backFace, setCode, langCode);
+        }
+
+        // No entry matched yet for edition
+        StaticData data = StaticData.instance();
+        CardEdition edition = data.getEditions().get(c.getEdition());
+        if (edition == null) // edition does not exist - some error occurred with card data
+            return null;
+        // 1. Try MCI code first, as it original.
+        String mciCode = edition.getMciCode().toLowerCase();
+        String url = ForgeConstants.URL_PIC_SCRYFALL_DOWNLOAD +
+                ImageUtil.getScryfallDownloadUrl(c, backFace, mciCode, langCode);
+        if (isValidScryfallURL(url)) {
+            scryfallSetCodes.put(c.getEdition(), setCode);
+            return url;
+        }
+        // 2. MCI didn't work, so now try all other codes available in edition, alias included.
+        // skipping dups with set, and returning as soon as one will work.
+        Set<String> cardSetCodes = new HashSet<>();
+        // all set-codes should be lower case
+        cardSetCodes.add(mciCode); // add MCI
+        cardSetCodes.add(edition.getCode().toLowerCase());
+        cardSetCodes.add(edition.getCode2().toLowerCase());
+        if (edition.getAlias() != null)
+            cardSetCodes.add(edition.getAlias().toLowerCase());
+        for (String code : cardSetCodes) {
+            if (code.equals(mciCode))
+                continue;  // Already checked, SKIP
+            url = ForgeConstants.URL_PIC_SCRYFALL_DOWNLOAD +
+                    ImageUtil.getScryfallDownloadUrl(c, backFace, code, langCode);
+            if (isValidScryfallURL(url)) {
+                scryfallSetCodes.put(c.getEdition(), setCode);
+                return url;
+            }
+        }
+        // If we're here, no valid URL has been found. Record this for the future
+        scryfallSetCodes.put(c.getEdition(), INVALID_SCRYFALL_SET_CODE);
+        return null;
+    }
+
     public void fetchImage(final String imageKey, final Callback callback) {
         FThreads.assertExecutedByEdt(true);
 
@@ -63,38 +125,24 @@ public abstract class ImageFetcher {
                 System.err.println("Paper card not found for: " + imageKey);
                 return;
             }
-            final boolean backFace = imageKey.endsWith(ImageKeys.BACKFACE_POSTFIX);
+            final boolean backFace = ImageUtil.hasBackFacePicture(paperCard);
             final String filename = ImageUtil.getImageKey(paperCard, backFace, true);
-            destFile = new File(ForgeConstants.CACHE_CARD_PICS_DIR + "/" + filename + ".jpg");
+            destFile = new File(ForgeConstants.CACHE_CARD_PICS_DIR, filename + ".jpg");
 
             //move priority of ftp image here
             StringBuilder setDownload = new StringBuilder(ForgeConstants.URL_PIC_DOWNLOAD);
             setDownload.append(ImageUtil.getDownloadUrl(paperCard, backFace));
             downloadUrls.add(setDownload.toString());
-
-            int artIndex = 1;
-            final Pattern pattern = Pattern.compile("^.:([^|]*\\|){2}(\\d+).*$");
-            Matcher matcher = pattern.matcher(imageKey);
-            if (matcher.matches()) {
-                artIndex = Integer.parseInt(matcher.group(2));
-            }
-            final StaticData data = StaticData.instance();
             final String cardCollectorNumber = paperCard.getCollectorNumber();
             if (!cardCollectorNumber.equals(IPaperCard.NO_COLLECTOR_NUMBER)){
-                String faceParam = "";
-                if (paperCard.getRules().getOtherPart() != null) {
-                    faceParam = (backFace ? "&face=back" : "&face=front");
-                }
-                final String editionMciCode = data.getEditions().getMciCodeByCode(paperCard.getEdition());
                 String langCode = "en";
                 String UILang = FModel.getPreferences().getPref(ForgePreferences.FPref.UI_LANGUAGE);
-                if (langCodeMap.containsKey(UILang)) {
+                if (langCodeMap.containsKey(UILang))
                     langCode = langCodeMap.get(UILang);
-                }
+                final String scryfallURL = this.getScryfallDownloadURL(paperCard, backFace, langCode);
-                // see https://scryfall.com/blog 2020/8/6, and
+                if (scryfallURL == null)
-                // https://scryfall.com/docs/api/cards/collector
+                    return;  // Non existing card, or Card's set not found in Scryfall
-                downloadUrls.add(String.format("https://api.scryfall.com/cards/%s/%s/%s?format=image&version=normal%s",
+                downloadUrls.add(scryfallURL);
-                        editionMciCode, cardCollectorNumber, langCode, faceParam));
             }
 
         } else if (prefix.equals(ImageKeys.TOKEN_PREFIX)) {
@@ -122,10 +170,11 @@ public abstract class ImageFetcher {
         }
 
         if (destFile.exists()) {
-            // TODO: Figure out why this codepath gets reached. Ideally, fetchImage() wouldn't
+            // TODO: Figure out why this codepath gets reached.
-            // be called if we already have the image.
+            //  Ideally, fetchImage() wouldn't be called if we already have the image.
             return;
         }
+
         final String destPath = destFile.getAbsolutePath();
 
         // Note: No synchronization is needed here because this is executed on