changeset 360:150500515714

Report non-critical errors during status operation to handler/inspector
author Artem Tikhomirov <tikhomirov.artem@gmail.com>
date Thu, 08 Dec 2011 22:19:27 +0100
parents 1d9bcab9c50f
children 8099939af5fa
files cmdline/org/tmatesoft/hg/console/Main.java cmdline/org/tmatesoft/hg/console/Status.java src/org/tmatesoft/hg/core/HgStatusCommand.java src/org/tmatesoft/hg/core/HgStatusHandler.java src/org/tmatesoft/hg/repo/HgChangelog.java src/org/tmatesoft/hg/repo/HgStatusCollector.java src/org/tmatesoft/hg/repo/HgStatusInspector.java src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java src/org/tmatesoft/hg/util/Status.java test/org/tmatesoft/hg/test/TestStatus.java
diffstat 10 files changed, 196 insertions(+), 12 deletions(-) [+]
line wrap: on
line diff
--- a/cmdline/org/tmatesoft/hg/console/Main.java	Thu Dec 08 15:34:13 2011 +0100
+++ b/cmdline/org/tmatesoft/hg/console/Main.java	Thu Dec 08 22:19:27 2011 +0100
@@ -640,6 +640,11 @@
 			}
 		}
 		
+		public void invalid(Path fname, Exception ex) {
+			System.out.printf("FAILURE: %s\n", fname);
+			ex.printStackTrace(System.out);
+		}
+		
 		private void print(char status, Path fname) {
 			if (!hideStatusPrefix) {
 				System.out.print(status);
--- a/cmdline/org/tmatesoft/hg/console/Status.java	Thu Dec 08 15:34:13 2011 +0100
+++ b/cmdline/org/tmatesoft/hg/console/Status.java	Thu Dec 08 22:19:27 2011 +0100
@@ -32,6 +32,7 @@
 import org.tmatesoft.hg.core.HgStatus;
 import org.tmatesoft.hg.core.HgStatus.Kind;
 import org.tmatesoft.hg.core.HgStatusCommand;
+import org.tmatesoft.hg.core.HgStatusHandler;
 import org.tmatesoft.hg.util.Path;
 
 /**
@@ -68,7 +69,7 @@
 //		cmd.subrepo(cmdLineOpts.getBoolean("-S", "--subrepos"))
 		final boolean noStatusPrefix = cmdLineOpts.getBoolean("-n", "--no-status");
 		final boolean showCopies = cmdLineOpts.getBoolean("-C", "--copies");
-		class StatusHandler implements HgStatusCommand.Handler {
+		class StatusHandler implements HgStatusHandler {
 			
 			final EnumMap<HgStatus.Kind, List<Path>> data = new EnumMap<HgStatus.Kind, List<Path>>(HgStatus.Kind.class);
 			final Map<Path, Path> copies = showCopies ? new HashMap<Path,Path>() : null;
@@ -85,6 +86,11 @@
 				}
 			}
 			
+			public void handleError(Path file, org.tmatesoft.hg.util.Status s) {
+				System.out.printf("FAILURE: %s %s\n", s.getMessage(), file);
+				s.getException().printStackTrace(System.out);
+			}
+			
 			public void dump() {
 				sortAndPrint('M', data.get(Kind.Modified), null);
 				sortAndPrint('A', data.get(Kind.Added), copies);
--- a/src/org/tmatesoft/hg/core/HgStatusCommand.java	Thu Dec 08 15:34:13 2011 +0100
+++ b/src/org/tmatesoft/hg/core/HgStatusCommand.java	Thu Dec 08 22:19:27 2011 +0100
@@ -30,6 +30,7 @@
 import org.tmatesoft.hg.repo.HgStatusInspector;
 import org.tmatesoft.hg.repo.HgWorkingCopyStatusCollector;
 import org.tmatesoft.hg.util.Path;
+import org.tmatesoft.hg.util.Status;
 
 /**
  * Command to obtain file status information, 'hg status' counterpart. 
@@ -164,7 +165,7 @@
 	 * @throws IllegalArgumentException if handler is <code>null</code>
 	 * @throws ConcurrentModificationException if this command already runs (i.e. being used from another thread)
 	 */
-	public void execute(Handler statusHandler) throws CancellationException, HgException, IOException {
+	public void execute(HgStatusHandler statusHandler) throws CancellationException, HgException, IOException {
 		if (statusHandler == null) {
 			throw new IllegalArgumentException();
 		}
@@ -189,12 +190,21 @@
 					sc.walk(startRevision, endRevision, mediator);
 				}
 			}
+		} catch (HgCallbackTargetException.Wrap ex) { 
+			// seems too general to catch RuntimeException, i.e.
+			// unless catch is for very narrow piece of code, it's better not to catch any RTE (which may happen elsewhere, not only in handler)
+			// XXX Perhaps, need more detailed explanation in handlers that are expected to throw Wrap/RTE (i.e. HgChangesetHandler)
+			throw new HgCallbackTargetException(ex).setRevisionNumber(endRevision);
 		} finally {
 			mediator.done();
 		}
 	}
 
-	public interface Handler {
+	/**
+	 * @deprecated replaced with {@link HgStatusHandler}
+	 */
+	@Deprecated
+	public interface Handler extends HgStatusHandler{
 		void handleStatus(HgStatus s);
 	}
 
@@ -207,13 +217,13 @@
 		boolean needClean;
 		boolean needIgnored;
 		boolean needCopies;
-		Handler handler;
+		HgStatusHandler handler;
 		private ChangelogHelper logHelper;
 
 		Mediator() {
 		}
 		
-		public void start(Handler h, ChangelogHelper changelogHelper) {
+		public void start(HgStatusHandler h, ChangelogHelper changelogHelper) {
 			handler = h;
 			logHelper = changelogHelper;
 		}
@@ -268,5 +278,9 @@
 				handler.handleStatus(new HgStatus(Ignored, fname, logHelper));
 			}
 		}
+		
+		public void invalid(Path fname, Exception ex) {
+			handler.handleError(fname, new Status(Status.Kind.ERROR, "Failed to get file status", ex));
+		}
 	}
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/src/org/tmatesoft/hg/core/HgStatusHandler.java	Thu Dec 08 22:19:27 2011 +0100
@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2011 TMate Software Ltd
+ *  
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * For information on how to redistribute this software under
+ * the terms of a license other than GNU General Public License
+ * contact TMate Software at support@hg4j.com
+ */
+package org.tmatesoft.hg.core;
+
+import org.tmatesoft.hg.util.Path;
+import org.tmatesoft.hg.util.Status;
+
+/**
+ * Callback to process {@link HgStatus} objects.
+ * @author Artem Tikhomirov
+ * @author TMate Software Ltd.
+ */
+public interface HgStatusHandler {
+
+	/* XXX #next() as in HgChangesetHandler?
+	 * perhaps, handle() is better name? If yes, rename method in HgChangesetHandler, too, to make them similar.
+	 * void next(HgStatus s);
+	 * XXX describe RTE and HgCallbackTargetException
+	 */
+	void handleStatus(HgStatus s);
+
+	/**
+	 * Report non-critical error processing single file during status operation
+	 * @param file name of the file that caused the trouble
+	 * @param s error description object
+	 */
+	void handleError(Path file, Status s);
+}
--- a/src/org/tmatesoft/hg/repo/HgChangelog.java	Thu Dec 08 15:34:13 2011 +0100
+++ b/src/org/tmatesoft/hg/repo/HgChangelog.java	Thu Dec 08 22:19:27 2011 +0100
@@ -238,6 +238,7 @@
 			}
 		}
 
+		// FIXME internal class DataAccess as part of API
 		public static RawChangeset parse(DataAccess da) {
 			try {
 				byte[] data = da.byteArray();
--- a/src/org/tmatesoft/hg/repo/HgStatusCollector.java	Thu Dec 08 15:34:13 2011 +0100
+++ b/src/org/tmatesoft/hg/repo/HgStatusCollector.java	Thu Dec 08 22:19:27 2011 +0100
@@ -271,9 +271,9 @@
 						inspector.added(copyTarget);
 					}
 				} catch (HgException ex) {
-					ex.printStackTrace();
-					// FIXME perhaps, shall record this exception to dedicated mediator and continue
+					// record exception to a mediator and continue, 
 					// for a single file not to be irresolvable obstacle for a status operation
+					inspector.invalid(r2fname, ex);
 				}
 			}
 		}
@@ -329,6 +329,7 @@
 	public static class Record implements HgStatusInspector {
 		private List<Path> modified, added, removed, clean, missing, unknown, ignored;
 		private Map<Path, Path> copied;
+		private Map<Path, Exception> failures;
 		
 		private int startRev, endRev;
 		private HgStatusCollector statusHelper;
@@ -402,6 +403,13 @@
 		public List<Path> getIgnored() {
 			return proper(ignored);
 		}
+
+		public Map<Path, Exception> getInvalid() {
+			if (failures == null) {
+				return Collections.emptyMap();
+			}
+			return Collections.unmodifiableMap(failures);
+		}
 		
 		private static List<Path> proper(List<Path> l) {
 			if (l == null) {
@@ -448,6 +456,13 @@
 		public void ignored(Path fname) {
 			ignored = doAdd(ignored, fname);
 		}
+		
+		public void invalid(Path fname, Exception ex) {
+			if (failures == null) {
+				failures = new LinkedHashMap<Path, Exception>();
+			}
+			failures.put(fname, ex);
+		}
 
 		private static List<Path> doAdd(List<Path> l, Path p) {
 			if (l == null) {
--- a/src/org/tmatesoft/hg/repo/HgStatusInspector.java	Thu Dec 08 15:34:13 2011 +0100
+++ b/src/org/tmatesoft/hg/repo/HgStatusInspector.java	Thu Dec 08 22:19:27 2011 +0100
@@ -35,7 +35,18 @@
 	void copied(Path fnameOrigin, Path fnameAdded);
 	void removed(Path fname);
 	void clean(Path fname);
-	void missing(Path fname); // aka deleted (tracked by Hg, but not available in FS any more
+	/**
+	 * Reports file tracked by Mercurial, but not available in file system any more, aka deleted. 
+	 */
+	void missing(Path fname); // 
 	void unknown(Path fname); // not tracked
 	void ignored(Path fname);
+	/**
+	 * Reports a single file error during status collecting operation. It's up to client to treat the whole operation as successful or not.
+	 * The error reported is otherwise not critical for the status operation.
+	 *  
+	 * @param fname origin of the error
+	 * @param ex describes an error occurred while accessing the file, never <code>null</code>
+	 */
+	void invalid(Path fname, Exception ex);
 }
\ No newline at end of file
--- a/src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java	Thu Dec 08 15:34:13 2011 +0100
+++ b/src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java	Thu Dec 08 22:19:27 2011 +0100
@@ -149,7 +149,7 @@
 		}
 		try {
 			if (getDirstateImpl() == null) {
-				// XXX this is a hack to avoid declaring throws for the #walk() at the moment
+				// FIXME this is a hack to avoid declaring throws for the #walk() at the moment
 				// once I decide whether to have mediator that collects errors or to use exceptions here
 				// this hack shall be removed in favor of either severe error in mediator or a re-thrown exception.
 					getDirstate();
@@ -339,8 +339,8 @@
 						return;
 					}
 				} catch (HgException ex) {
-					ex.printStackTrace();
-					// FIXME report to a mediator, continue status collection
+					// report failure and continue status collection
+					inspector.invalid(fname, ex);
 				}
 			} else if ((r = getDirstateImpl().checkAdded(fname)) != null) {
 				if (r.copySource() != null && baseRevNames.contains(r.copySource())) {
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/src/org/tmatesoft/hg/util/Status.java	Thu Dec 08 22:19:27 2011 +0100
@@ -0,0 +1,63 @@
+/*
+ * Copyright (c) 2011 TMate Software Ltd
+ *  
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * For information on how to redistribute this software under
+ * the terms of a license other than GNU General Public License
+ * contact TMate Software at support@hg4j.com
+ */
+package org.tmatesoft.hg.util;
+
+import org.tmatesoft.hg.internal.Experimental;
+
+/**
+ * Success/failure descriptor. When exception is too much.
+ * 
+ * @author Artem Tikhomirov
+ * @author TMate Software Ltd.
+ */
+@Experimental(reason="Accidental use, does not justify dedicated class, perhaps.")
+public class Status {
+	// XXX perhaps private enum and factory method createError() and createOk()?
+	public enum Kind {
+		OK, ERROR;
+	}
+
+	private final Kind kind;
+	private final String message;
+	private final Exception error;
+
+	public Status(Kind k, String msg) {
+		this(k, msg, null);
+	}
+	
+	public Status(Kind k, String msg, Exception err) {
+		kind = k;
+		message = msg;
+		error = err;
+	}
+	
+	public boolean isOk() {
+		return kind == Kind.OK;
+	}
+	
+	public Kind getKind() {
+		return kind;
+	}
+
+	public String getMessage() {
+		return message;
+	}
+	
+	public Exception getException() {
+		return error;
+	}
+}
--- a/test/org/tmatesoft/hg/test/TestStatus.java	Thu Dec 08 15:34:13 2011 +0100
+++ b/test/org/tmatesoft/hg/test/TestStatus.java	Thu Dec 08 22:19:27 2011 +0100
@@ -26,6 +26,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -38,12 +39,14 @@
 import org.tmatesoft.hg.core.HgStatus;
 import org.tmatesoft.hg.core.HgStatus.Kind;
 import org.tmatesoft.hg.core.HgStatusCommand;
+import org.tmatesoft.hg.core.HgStatusHandler;
 import org.tmatesoft.hg.internal.PathGlobMatcher;
 import org.tmatesoft.hg.repo.HgLookup;
 import org.tmatesoft.hg.repo.HgRepository;
 import org.tmatesoft.hg.repo.HgStatusCollector;
 import org.tmatesoft.hg.repo.HgWorkingCopyStatusCollector;
 import org.tmatesoft.hg.util.Path;
+import org.tmatesoft.hg.util.Status;
 
 
 /**
@@ -173,9 +176,10 @@
 		// TODO check not -A, but defaults()/custom set of modifications 
 	}
 	
-	private static class StatusCollector implements HgStatusCommand.Handler {
+	private static class StatusCollector implements HgStatusHandler {
 		private final Map<Kind, List<Path>> kind2names = new TreeMap<Kind, List<Path>>();
 		private final Map<Path, List<Kind>> name2kinds = new TreeMap<Path, List<Kind>>();
+		private final Map<Path, Status> name2error = new LinkedHashMap<Path, Status>();
 
 		public void handleStatus(HgStatus s) {
 			List<Path> l = kind2names.get(s.getKind());
@@ -191,6 +195,10 @@
 			k.add(s.getKind());
 		}
 		
+		public void handleError(Path file, Status s) {
+			name2error.put(file, s);
+		}
+		
 		public List<Path> get(Kind k) {
 			List<Path> rv = kind2names.get(k);
 			return rv == null ? Collections.<Path>emptyList() : rv;
@@ -200,6 +208,10 @@
 			List<Kind> rv = name2kinds.get(p);
 			return rv == null ? Collections.<Kind>emptyList() : rv;
 		}
+		
+		public Map<Path, Status> getErrors() {
+			return name2error;
+		}
 	}
 
 	/*
@@ -213,6 +225,7 @@
 		HgStatusCommand cmd = new HgStatusCommand(repo);
 		StatusCollector sc = new StatusCollector();
 		cmd.all().base(7).execute(sc);
+		assertTrue(sc.getErrors().isEmpty());
 		Path file5 = Path.create("dir/file5");
 		// shall not be listed at all
 		assertTrue(sc.get(file5).isEmpty());
@@ -230,6 +243,7 @@
 		HgStatusCommand cmd = new HgStatusCommand(repo);
 		StatusCollector sc = new StatusCollector();
 		cmd.all().execute(sc);
+		assertTrue(sc.getErrors().isEmpty());
 		final Path file2 = Path.create("file2");
 		assertTrue(sc.get(file2).contains(Modified));
 		assertTrue(sc.get(file2).size() == 1);
@@ -246,17 +260,20 @@
 		HgStatusCommand cmd = new HgStatusCommand(repo);
 		StatusCollector sc = new StatusCollector();
 		cmd.all().execute(sc);
+		assertTrue(sc.getErrors().isEmpty());
 		Path file4 = Path.create("dir/file4");
 		assertTrue(sc.get(file4).contains(Removed));
 		assertTrue(sc.get(file4).size() == 1);
 		//
 		// different code path (collect != null)
 		cmd.base(3).execute(sc = new StatusCollector());
+		assertTrue(sc.getErrors().isEmpty());
 		assertTrue(sc.get(file4).contains(Removed));
 		assertTrue(sc.get(file4).size() == 1);
 		//
 		// wasn't there in rev 2, shall not be reported at all
 		cmd.base(2).execute(sc = new StatusCollector());
+		assertTrue(sc.getErrors().isEmpty());
 		assertTrue(sc.get(file4).isEmpty());
 	}
 
@@ -273,20 +290,24 @@
 		HgStatusCommand cmd = new HgStatusCommand(repo);
 		StatusCollector sc = new StatusCollector();
 		cmd.all().execute(sc);
+		assertTrue(sc.getErrors().isEmpty());
 		final Path file3 = Path.create("dir/file3");
 		assertTrue(sc.get(file3).contains(Ignored));
 		assertTrue(sc.get(file3).size() == 1);
 		//
 		cmd.base(3).execute(sc = new StatusCollector());
+		assertTrue(sc.getErrors().isEmpty());
 		assertTrue(sc.get(file3).contains(Ignored));
 		assertTrue(sc.get(file3).contains(Removed));
 		assertTrue(sc.get(file3).size() == 2);
 		//
 		cmd.base(5).execute(sc = new StatusCollector());
+		assertTrue(sc.getErrors().isEmpty());
 		assertTrue(sc.get(file3).contains(Ignored));
 		assertTrue(sc.get(file3).size() == 1);
 		//
 		cmd.base(0).execute(sc = new StatusCollector());
+		assertTrue(sc.getErrors().isEmpty());
 		assertTrue(sc.get(file3).contains(Ignored));
 		assertTrue(sc.get(file3).size() == 1);
 
@@ -304,6 +325,7 @@
 		StatusCollector sc = new StatusCollector();
 		cmd.base(1);
 		cmd.all().execute(sc);
+		assertTrue(sc.getErrors().isEmpty());
 		final Path file1 = Path.create("file1");
 		assertTrue(sc.get(file1).contains(Unknown));
 		assertTrue(sc.get(file1).contains(Removed));
@@ -311,6 +333,7 @@
 		// 
 		// no file1 in rev 2, shall be reported as unknown only
 		cmd.base(2).execute(sc = new StatusCollector());
+		assertTrue(sc.getErrors().isEmpty());
 		assertTrue(sc.get(file1).contains(Unknown));
 		assertTrue(sc.get(file1).size() == 1);
 	}
@@ -322,6 +345,7 @@
 		StatusCollector sc = new StatusCollector();
 		cmd.match(new PathGlobMatcher("*"));
 		cmd.all().execute(sc);
+		assertTrue(sc.getErrors().isEmpty());
 		/*
 		 * C .hgignore
 		 * ? file1
@@ -336,6 +360,7 @@
 		assertTrue(sc.get(Modified).size() == 1);
 		//
 		cmd.match(new PathGlobMatcher("dir/*")).execute(sc = new StatusCollector());
+		assertTrue(sc.getErrors().isEmpty());
 		/*
 		 * I dir/file3
 		 * R dir/file4
@@ -414,6 +439,7 @@
 		cmd.match(new PathGlobMatcher("dir/*"));
 		StatusCollector sc = new StatusCollector();
 		cmd.execute(sc);
+		assertTrue(sc.getErrors().isEmpty());
 		final Path file3 = Path.create("dir/file3");
 		final Path file4 = Path.create("dir/file4");
 		final Path file5 = Path.create("dir/file5");
@@ -464,6 +490,7 @@
 	}
 	
 	private void report(String what, StatusCollector r) {
+		assertTrue(r.getErrors().isEmpty());
 		reportNotEqual(what + "#MODIFIED", r.get(Modified), statusParser.getModified());
 		reportNotEqual(what + "#ADDED", r.get(Added), statusParser.getAdded());
 		reportNotEqual(what + "#REMOVED", r.get(Removed), statusParser.getRemoved());