]> source.dussan.org Git - jgit.git/commitdiff
Revive Repository#notifyIndexChanged() 70/122970/1
authorJonathan Nieder <jrn@google.com>
Fri, 18 May 2018 16:15:30 +0000 (09:15 -0700)
committerJonathan Nieder <jrn@google.com>
Fri, 18 May 2018 16:15:30 +0000 (09:15 -0700)
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 <jrn@google.com>
org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java

index e7e2b003493e01a8e351710c101f006ba7f72940..965ccb0989d7732238e831f0c1337e623a77d6a2 100644 (file)
@@ -112,6 +112,12 @@ import org.slf4j.LoggerFactory;
  * <li>{@code FileRepository} is thread-safe.
  * <li>{@code DfsRepository} thread-safety is determined by its subclass.
  * </ul>
+ * <p>
+ * <i>Note to implementors:</i> 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.
+        * <p>
+        * 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