From ca79b3d4af748c626d2b35d7404ef49983dfe917 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 18 May 2018 09:15:30 -0700 Subject: [PATCH] Revive Repository#notifyIndexChanged() e9e150fdd24d (Store in IndexChangedEvent if it was caused by JGit itself, 2018-05-13) modified Repository#notifyIndexChanged to take a boolean argument to indicate whether the index change happened under the current process's control or externally, for use by EGit. In other words, the function signature changed from public abstract void notifyIndexChanged(); to public abstract void notifyIndexChanged(boolean internal); Callers outside JGit itself notifying a Repository about index changes are expected to be rare, so this is not very disruptive to them. In most cases they would be notifying about changes that they made themselves, so treating their notifyIndexChanged() calls as notifyIndexChanged(true) should be relatively safe. Implementors have the opposite problem: adding the new "abstract void notifyIndexChanged(boolean)" method means they are obligated to override it. Add a default implementation that calls their existing override of notifyIndexChanged() to make their migration easier. The main downside is that authors of new Repository subclasses that do not realize they need to override notifyIndexChanged would end up with a default implementation which calls notifyIndexChanged(true), in turn calling notifyIndexChanged() again and so on, resulting in StackOverflowException. Add an implementors' note to the class Javadoc to avoid this issue. A followup commit will force implementors to adapt to the new API by changing the methods to @Deprecated public final void notifyIndexChanged() { notifyIndexChanged(true); } public abstract void notifyIndexChanged(boolean internal); Change-Id: I7d014890ee19abf283ea824d9baa9044bfdde130 Signed-off-by: Jonathan Nieder --- .../src/org/eclipse/jgit/lib/Repository.java | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java index e7e2b0034..965ccb098 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java @@ -112,6 +112,12 @@ import org.slf4j.LoggerFactory; *
  • {@code FileRepository} is thread-safe. *
  • {@code DfsRepository} thread-safety is determined by its subclass. * + *

    + * Note to implementors: Make sure to override + * {@link #notifyIndexChanged(boolean)} or {@link #notifyIndexChanged()}, or + * else both will throw {@code StackOverflowException}. In the next JGit minor + * release, {@link #notifyIndexChanged(boolean)} will be abstract and {@link + * #notifyIndexChanged()} will be final. */ public abstract class Repository implements AutoCloseable { private static final Logger LOG = LoggerFactory.getLogger(Repository.class); @@ -1567,15 +1573,31 @@ public abstract class Repository implements AutoCloseable { */ public abstract void scanForRepoChanges() throws IOException; + /** + * Backward compatibility synonym for {@code notifyIndexChanged(true)}. + * + * @deprecated replaced by {@link #notifyIndexChanged(boolean)} + */ + @Deprecated + public void notifyIndexChanged() { + notifyIndexChanged(true); + } + /** * Notify that the index changed by firing an IndexChangedEvent. + *

    + * The default implementation calls {@link #notifyIndexChanged()} for + * backward compatibility but will no longer do so in the next JGit minor + * release. Implementors should override this method directly instead. * * @param internal * {@code true} if the index was changed by the same * JGit process * @since 5.0 */ - public abstract void notifyIndexChanged(boolean internal); + public void notifyIndexChanged(boolean internal) { + notifyIndexChanged(); + } /** * Get a shortened more user friendly ref name