From 3ff78c2cbf1a81d6012e31cd0083e1ce9c3f346c Mon Sep 17 00:00:00 2001 From: Peter Osterlund Date: Fri, 27 Sep 2019 21:26:41 +0200 Subject: [PATCH] Better handling of IO errors when reading/writing PGN files --- .../droidfish/gamelogic/PGNFileTest.java | 20 ++----- .../java/org/petero/droidfish/DroidFish.java | 14 +++-- .../petero/droidfish/activities/EditPGN.java | 51 +++++++++-------- .../petero/droidfish/activities/PGNFile.java | 56 +++++++++---------- 4 files changed, 68 insertions(+), 73 deletions(-) diff --git a/DroidFishApp/src/androidTest/java/org/petero/droidfish/gamelogic/PGNFileTest.java b/DroidFishApp/src/androidTest/java/org/petero/droidfish/gamelogic/PGNFileTest.java index a69e176..155d488 100644 --- a/DroidFishApp/src/androidTest/java/org/petero/droidfish/gamelogic/PGNFileTest.java +++ b/DroidFishApp/src/androidTest/java/org/petero/droidfish/gamelogic/PGNFileTest.java @@ -18,8 +18,6 @@ package org.petero.droidfish.gamelogic; -import android.util.Pair; - import java.io.File; import java.io.FileOutputStream; import java.io.IOException; @@ -28,7 +26,6 @@ import java.util.ArrayList; import org.petero.droidfish.DroidFishApp; import org.petero.droidfish.activities.PGNFile; import org.petero.droidfish.activities.PGNFile.GameInfo; -import org.petero.droidfish.activities.PGNFile.GameInfoResult; import junit.framework.TestCase; @@ -47,9 +44,7 @@ public class PGNFileTest extends TestCase { }; writeFile(f, lines); PGNFile pgnFile = new PGNFile(f.getAbsolutePath()); - Pair> res = pgnFile.getGameInfo(null, null); - assertEquals(GameInfoResult.OK, res.first); - ArrayList gi = res.second; + ArrayList gi = pgnFile.getGameInfo(null, null); assertEquals(1, gi.size()); assertEquals(0, gi.get(0).startPos); assertEquals(14, gi.get(0).endPos); @@ -97,9 +92,7 @@ public class PGNFileTest extends TestCase { }; writeFile(f, lines); PGNFile pgnFile = new PGNFile(f.getAbsolutePath()); - Pair> res = pgnFile.getGameInfo(null, null); - assertEquals(GameInfoResult.OK, res.first); - ArrayList gi = res.second; + ArrayList gi = pgnFile.getGameInfo(null, null); assertEquals(2, gi.size()); assertEquals(0, gi.get(0).startPos); assertEquals(660, gi.get(0).endPos); @@ -127,9 +120,7 @@ public class PGNFileTest extends TestCase { }; writeFile(f, lines); PGNFile pgnFile = new PGNFile(f.getAbsolutePath()); - Pair> res = pgnFile.getGameInfo(null, null); - assertEquals(GameInfoResult.OK, res.first); - ArrayList gi = res.second; + ArrayList gi = pgnFile.getGameInfo(null, null); assertEquals(2, gi.size()); assertEquals(4, gi.get(0).startPos); assertEquals(80, gi.get(0).endPos); @@ -138,9 +129,8 @@ public class PGNFileTest extends TestCase { assertEquals(137, gi.get(1).endPos); assertEquals("2. w - b 1-0", gi.get(1).info); - res = pgnFile.getGameInfo(1); - assertEquals(GameInfoResult.OK, res.first); - assertEquals(1, res.second.size()); + gi = pgnFile.getGameInfo(1); + assertEquals(1, gi.size()); } } diff --git a/DroidFishApp/src/main/java/org/petero/droidfish/DroidFish.java b/DroidFishApp/src/main/java/org/petero/droidfish/DroidFish.java index ecd34e4..f8dc623 100644 --- a/DroidFishApp/src/main/java/org/petero/droidfish/DroidFish.java +++ b/DroidFishApp/src/main/java/org/petero/droidfish/DroidFish.java @@ -46,7 +46,6 @@ import org.petero.droidfish.activities.LoadFEN; import org.petero.droidfish.activities.LoadScid; import org.petero.droidfish.activities.PGNFile; import org.petero.droidfish.activities.PGNFile.GameInfo; -import org.petero.droidfish.activities.PGNFile.GameInfoResult; import org.petero.droidfish.activities.Preferences; import org.petero.droidfish.book.BookOptions; import org.petero.droidfish.engine.EngineUtil; @@ -798,8 +797,13 @@ public class DroidFish extends Activity } PGNFile pgnFile = new PGNFile(fn); long fileLen = FileUtil.getFileLength(fn); - Pair> gi = pgnFile.getGameInfo(2); - if ((fileLen > 1024 * 1024) || (gi.first == GameInfoResult.OK && gi.second.size() > 1)) { + boolean moreThanOneGame = false; + try { + ArrayList gi = pgnFile.getGameInfo(2); + moreThanOneGame = gi.size() > 1; + } catch (IOException ignore) { + } + if (fileLen > 1024 * 1024 || moreThanOneGame) { filename = fn; } else { try (FileInputStream in = new FileInputStream(fn)) { @@ -2288,8 +2292,8 @@ public class DroidFish extends Activity fenPgn.append(clip.getItemAt(i).coerceToText(getApplicationContext())); try { String fenPgnData = fenPgn.toString(); - Pair> gi = PGNFile.getGameInfo(fenPgnData, 2); - if (gi.first == GameInfoResult.OK && gi.second.size() > 1) { + ArrayList gi = PGNFile.getGameInfo(fenPgnData, 2); + if (gi.size() > 1) { String sep = File.separator; String fn = Environment.getExternalStorageDirectory() + sep + pgnDir + sep + ".sharedfile.pgn"; diff --git a/DroidFishApp/src/main/java/org/petero/droidfish/activities/EditPGN.java b/DroidFishApp/src/main/java/org/petero/droidfish/activities/EditPGN.java index a6b1214..02d56cd 100644 --- a/DroidFishApp/src/main/java/org/petero/droidfish/activities/EditPGN.java +++ b/DroidFishApp/src/main/java/org/petero/droidfish/activities/EditPGN.java @@ -50,10 +50,11 @@ import org.petero.droidfish.ObjectCache; import org.petero.droidfish.R; import org.petero.droidfish.Util; import org.petero.droidfish.activities.PGNFile.GameInfo; -import org.petero.droidfish.activities.PGNFile.GameInfoResult; import org.petero.droidfish.databinding.SelectGameBinding; import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; import java.util.ArrayList; import java.util.Locale; @@ -180,7 +181,7 @@ public abstract class EditPGN extends AppCompatActivity { if (canceled) { setResult(RESULT_CANCELED); finish(); - } else if (gamesInFile.size() == 0) { + } else if (gamesInFile.isEmpty()) { pgnFile.appendPGN(pgnToSave); finish(); } else { @@ -431,29 +432,33 @@ public abstract class EditPGN extends AppCompatActivity { long modTime = new File(fileName).lastModified(); if (cacheValid && (modTime == lastModTime) && fileName.equals(lastFileName)) return true; - Pair> p = pgnFile.getGameInfo(this, progress); - if (p.first != GameInfoResult.OK) { - gamesInFile = new ArrayList<>(); - switch (p.first) { - case OUT_OF_MEMORY: - runOnUiThread(() -> DroidFishApp.toast(R.string.file_too_large, Toast.LENGTH_SHORT)); - break; - case NOT_PGN: - runOnUiThread(() -> DroidFishApp.toast(R.string.not_a_pgn_file, Toast.LENGTH_SHORT)); - break; - case CANCEL: - case OK: - break; + try { + gamesInFile = pgnFile.getGameInfo(this, progress); + cacheValid = true; + lastModTime = modTime; + lastFileName = fileName; + return true; + } catch (PGNFile.CancelException ignore) { + } catch (PGNFile.NotPgnFile ex) { + runOnUiThread(() -> DroidFishApp.toast(R.string.not_a_pgn_file, + Toast.LENGTH_SHORT)); + } catch (FileNotFoundException ex) { + if (!loadGame) { + gamesInFile = new ArrayList<>(); + return true; } - setResult(RESULT_CANCELED); - finish(); - return false; + runOnUiThread(() -> DroidFishApp.toast(ex.getMessage(), + Toast.LENGTH_LONG)); + } catch (IOException ex) { + runOnUiThread(() -> DroidFishApp.toast(ex.getMessage(), + Toast.LENGTH_LONG)); + } catch (OutOfMemoryError ex) { + runOnUiThread(() -> DroidFishApp.toast(R.string.file_too_large, + Toast.LENGTH_SHORT)); } - gamesInFile = p.second; - cacheValid = true; - lastModTime = modTime; - lastFileName = fileName; - return true; + setResult(RESULT_CANCELED); + finish(); + return false; } private void sendBackResult(GameInfo gi) { diff --git a/DroidFishApp/src/main/java/org/petero/droidfish/activities/PGNFile.java b/DroidFishApp/src/main/java/org/petero/droidfish/activities/PGNFile.java index ee4c30b..f6adb11 100644 --- a/DroidFishApp/src/main/java/org/petero/droidfish/activities/PGNFile.java +++ b/DroidFishApp/src/main/java/org/petero/droidfish/activities/PGNFile.java @@ -33,7 +33,6 @@ import org.petero.droidfish.R; import android.app.Activity; import android.app.ProgressDialog; -import android.util.Pair; import android.widget.Toast; public class PGNFile { @@ -111,13 +110,6 @@ public class PGNFile { } } - public static enum GameInfoResult { - OK, - CANCEL, - NOT_PGN, - OUT_OF_MEMORY; - } - private static class BytesToString { private byte[] buf = new byte[256]; private int len = 0; @@ -185,9 +177,21 @@ public class PGNFile { } } + public static class NotPgnFile extends IOException { + NotPgnFile() { + super(""); + } + } + + public static class CancelException extends IOException { + CancelException() { + super(""); + } + } + /** Return info about all PGN games in a file. */ - public Pair> getGameInfo(Activity activity, - ProgressDialog progress) { + public ArrayList getGameInfo(Activity activity, + ProgressDialog progress) throws IOException { if (activity == null || progress == null) return getGameInfoFromFile(null, -1); ProgressHandler handler = new ProgressHandler(fileName, activity, progress); @@ -195,41 +199,36 @@ public class PGNFile { } /** Return info about up to "maxGames" PGN games in a file. */ - public Pair> getGameInfo(int maxGames) { + public ArrayList getGameInfo(int maxGames) throws IOException { return getGameInfoFromFile(null, maxGames); } - public static Pair> getGameInfo(String pgnData, - int maxGames) { + public static ArrayList getGameInfo(String pgnData, int maxGames) { try (InputStream is = new ByteArrayInputStream(pgnData.getBytes("UTF-8"))) { return getGameInfo(is, null, maxGames); } catch (IOException ex) { - return new Pair<>(GameInfoResult.NOT_PGN, null); + return new ArrayList<>(); } } - private Pair> getGameInfoFromFile(ProgressHandler progress, - int maxGames) { + private ArrayList getGameInfoFromFile(ProgressHandler progress, + int maxGames) throws IOException { try (InputStream is = new FileInputStream(fileName)) { return getGameInfo(is, progress, maxGames); - } catch (IOException ex) { - return new Pair<>(GameInfoResult.NOT_PGN, null); } } - /** Return info about PGN games in a file. */ - private static Pair> getGameInfo(InputStream is, - ProgressHandler progress, - int maxGames) { + private static ArrayList getGameInfo(InputStream is, ProgressHandler progress, + int maxGames) throws IOException { ArrayList gamesInFile = new ArrayList<>(); + long nRead = 0; try (BufferedInput f = new BufferedInput(is)) { GameInfo gi = null; HeaderInfo hi = null; boolean inHeader = false; boolean inHeaderSection = false; long filePos = 0; - long nRead = 0; int gameNo = 1; final int INITIAL = 0; @@ -383,7 +382,7 @@ public class PGNFile { if (progress != null) progress.reportProgress(filePos); if (Thread.currentThread().isInterrupted()) - return new Pair<>(GameInfoResult.CANCEL, null); + throw new CancelException(); } gi = new GameInfo(); gi.startPos = filePos; @@ -397,14 +396,11 @@ public class PGNFile { gi.info = hi.toString(); gamesInFile.add(gi); } - } catch (IOException ignore) { - } catch (OutOfMemoryError e) { - return new Pair<>(GameInfoResult.OUT_OF_MEMORY, null); } - if (gamesInFile.isEmpty()) - return new Pair<>(GameInfoResult.NOT_PGN, null); + if (gamesInFile.isEmpty() && nRead > 1) + throw new NotPgnFile(); - return new Pair<>(GameInfoResult.OK, gamesInFile); + return gamesInFile; } private void mkDirs() {