Allow ArchiveCommand.registerFormat to be called twice

This should make it possible for the gitiles plugin to register its
archive formats after gerrit has already registered them.

Signed-off-by: Jonathan Nieder <jrn@google.com>
Change-Id: Icb80a446e583961a7278b707d572d6fe456c372c

org.eclipse.jgit.archive/src/org/eclipse/jgit/archive/ArchiveFormats.java

@@ -66,7 +66,6 @@ private static final void register(String name, ArchiveCommand.Format<?> fmt) {
 	 * Register all included archive formats so they can be used
 	 * as arguments to the ArchiveCommand.setFormat() method.
 	 *
-	 * Should not be called twice without a call to stop() in between.
 	 * Not thread-safe.
 	 */
 	public static void registerAll() {

org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties

@@ -12,7 +12,7 @@ anExceptionOccurredWhileTryingToAddTheIdOfHEAD=An exception occurred while tryin
 anSSHSessionHasBeenAlreadyCreated=An SSH session has been already created
 applyingCommit=Applying {0}
 archiveFormatAlreadyAbsent=Archive format already absent: {0}
-archiveFormatAlreadyRegistered=Archive format already registered: {0}
+archiveFormatAlreadyRegistered=Archive format already registered with different implementation: {0}
 argumentIsNotAValidCommentString=Invalid comment: {0}
 atLeastOnePathIsRequired=At least one path is required.
 atLeastOnePatternIsRequired=At least one pattern is required.

org.eclipse.jgit/src/org/eclipse/jgit/api/ArchiveCommand.java

@@ -189,65 +189,135 @@ public String getFormat() {
 		}
 	}
 
+	private static class FormatEntry {
+		final Format<?> format;
+		/** Number of times this format has been registered. */
+		final int refcnt;
+
+		public FormatEntry(Format<?> format, int refcnt) {
+			if (format == null)
+				throw new NullPointerException();
+			this.format = format;
+			this.refcnt = refcnt;
+		}
+	};
+
 	/**
 	 * Available archival formats (corresponding to values for
 	 * the --format= option)
 	 */
-	private static final ConcurrentMap<String, Format<?>> formats =
+	private static final ConcurrentMap<String, FormatEntry> formats =
-			new ConcurrentHashMap<String, Format<?>>();
+			new ConcurrentHashMap<String, FormatEntry>();
+
+	/**
+	 * Replaces the entry for a key only if currently mapped to a given
+	 * value.
+	 *
+	 * @param map a map
+	 * @param key key with which the specified value is associated
+	 * @param oldValue expected value for the key (null if should be absent).
+	 * @param newValue value to be associated with the key (null to remove).
+	 * @return true if the value was replaced
+	 */
+	private static <K, V> boolean replace(ConcurrentMap<K, V> map,
+			K key, V oldValue, V newValue) {
+		if (oldValue == null && newValue == null) // Nothing to do.
+			return true;
+
+		if (oldValue == null)
+			return map.putIfAbsent(key, newValue) == null;
+		else if (newValue == null)
+			return map.remove(key, oldValue);
+		else
+			return map.replace(key, oldValue, newValue);
+	}
 
 	/**
 	 * Adds support for an additional archival format.  To avoid
 	 * unnecessary dependencies, ArchiveCommand does not have support
 	 * for any formats built in; use this function to add them.
-	 *
+	 * <p>
 	 * OSGi plugins providing formats should call this function at
 	 * bundle activation time.
+	 * <p>
+	 * It is okay to register the same archive format with the same
+	 * name multiple times, but don't forget to unregister it that
+	 * same number of times, too.
+	 * <p>
+	 * Registering multiple formats with different names and the
+	 * same or overlapping suffixes results in undefined behavior.
+	 * TODO: check that suffixes don't overlap.
 	 *
 	 * @param name name of a format (e.g., "tar" or "zip").
 	 * @param fmt archiver for that format
 	 * @throws JGitInternalException
-	 *              An archival format with that name was already registered.
+	 *              A different archival format with that name was
+	 *              already registered.
 	 */
 	public static void registerFormat(String name, Format<?> fmt) {
-		// TODO(jrn): Check that suffixes don't overlap.
+		if (fmt == null)
+			throw new NullPointerException();
 
-		if (formats.putIfAbsent(name, fmt) != null)
+		FormatEntry old, entry;
-			throw new JGitInternalException(MessageFormat.format(
+		do {
-					JGitText.get().archiveFormatAlreadyRegistered,
+			old = formats.get(name);
-					name));
+			if (old == null) {
+				entry = new FormatEntry(fmt, 1);
+				continue;
+			}
+			if (!old.format.equals(fmt))
+				throw new JGitInternalException(MessageFormat.format(
+						JGitText.get().archiveFormatAlreadyRegistered,
+						name));
+			entry = new FormatEntry(old.format, old.refcnt + 1);
+		} while (!replace(formats, name, old, entry));
 	}
 
 	/**
-	 * Removes support for an archival format so its Format can be
+	 * Marks support for an archival format as no longer needed so its
-	 * garbage collected.
+	 * Format can be garbage collected if no one else is using it either.
+	 * <p>
+	 * In other words, this decrements the reference count for an
+	 * archival format.  If the reference count becomes zero, removes
+	 * support for that format.
 	 *
 	 * @param name name of format (e.g., "tar" or "zip").
 	 * @throws JGitInternalException
 	 *              No such archival format was registered.
 	 */
 	public static void unregisterFormat(String name) {
-		if (formats.remove(name) == null)
+		FormatEntry old, entry;
-			throw new JGitInternalException(MessageFormat.format(
+		do {
-					JGitText.get().archiveFormatAlreadyAbsent,
+			old = formats.get(name);
-					name));
+			if (old == null)
+				throw new JGitInternalException(MessageFormat.format(
+						JGitText.get().archiveFormatAlreadyAbsent,
+						name));
+			if (old.refcnt == 1) {
+				entry = null;
+				continue;
+			}
+			entry = new FormatEntry(old.format, old.refcnt - 1);
+		} while (!replace(formats, name, old, entry));
 	}
 
 	private static Format<?> formatBySuffix(String filenameSuffix)
 			throws UnsupportedFormatException {
 		if (filenameSuffix != null)
-			for (Format<?> fmt : formats.values())
+			for (FormatEntry entry : formats.values()) {
+				Format<?> fmt = entry.format;
 				for (String sfx : fmt.suffixes())
 					if (filenameSuffix.endsWith(sfx))
 						return fmt;
+			}
 		return lookupFormat("tar"); //$NON-NLS-1$
 	}
 
 	private static Format<?> lookupFormat(String formatName) throws UnsupportedFormatException {
-		Format<?> fmt = formats.get(formatName);
+		FormatEntry entry = formats.get(formatName);
-		if (fmt == null)
+		if (entry == null)
 			throw new UnsupportedFormatException(formatName);
-		return fmt;
+		return entry.format;
 	}
 
 	private OutputStream out;