Bug 471861 - The "eclipse.p2.max.threads" property has no effect forI20191013-1800I20191012-1800

values greater than four

Update SimpleArtifactRepositoryTest.testMaximumThreads() to test for the
modified expected behavior. I.e., that the repo can specify a maximum
that must be respected, the user can specify a maximum that is respected
if the repo doesn't specify a lower maximum, and if the user doesn't
specify a maximum, it behaves the same way as if the user specified a
maximum of 4.

Change-Id: I64b80d04b982ef37e60cf07312084ffaaab7b05a
Signed-off-by: Ed Merks <ed.merks@gmail.com>

2 files changed, 29 insertions, 12 deletions

diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java
index 6143ba749..992beed7d 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java
@@ -955,24 +955,26 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 	/**
 	 * Returns the maximum number of concurrent download threads.
 	 */
+
 	private int getMaximumThreads() {
-		int repoMaxThreads = DEFAULT_MAX_THREADS;
-		int userMaxThreads = DEFAULT_MAX_THREADS;
+		int maxThreads = DEFAULT_MAX_THREADS;
 		try {
-			String maxThreadString = getProperties().get(PROP_MAX_THREADS);
+			String maxThreadString = Activator.getContext().getProperty(PROP_MAX_THREADS);
 			if (maxThreadString != null)
-				repoMaxThreads = Math.max(1, Integer.parseInt(maxThreadString));
+				maxThreads = Math.max(1, Integer.parseInt(maxThreadString));
 		} catch (NumberFormatException nfe) {
 			// default number of threads
 		}
 		try {
-			String maxThreadString = Activator.getContext().getProperty(PROP_MAX_THREADS);
-			if (maxThreadString != null)
-				userMaxThreads = Math.max(1, Integer.parseInt(maxThreadString));
+			String maxThreadString = getProperties().get(PROP_MAX_THREADS);
+			if (maxThreadString != null) {
+				int repoMaxThreads = Math.max(1, Integer.parseInt(maxThreadString));
+				maxThreads = Math.min(maxThreads, repoMaxThreads);
+			}
 		} catch (NumberFormatException nfe) {
-			// default number of threads
+			// ignore repoMaxThreads
 		}
-		return Math.min(repoMaxThreads, userMaxThreads);
+		return maxThreads;
 	}
 
 	@Override
diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/SimpleArtifactRepositoryTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/SimpleArtifactRepositoryTest.java
index f3bd0ee5d..e8eaee4a8 100644
--- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/SimpleArtifactRepositoryTest.java
+++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/SimpleArtifactRepositoryTest.java
@@ -474,24 +474,39 @@ public class SimpleArtifactRepositoryTest extends AbstractProvisioningTest {
 		System.setProperty(SimpleArtifactRepository.PROP_MAX_THREADS, "2");
 		assertEquals("Valid User setting", 2, getIntVal(getMaximumThreads, repo));
 
-		// User value is too high
+		// User value is high, but repo specifies no limit, so user value is used
+		// directly.
 		System.setProperty(SimpleArtifactRepository.PROP_MAX_THREADS, "22");
-		assertEquals("Invalid User setting", defaultMaxThreads, getIntVal(getMaximumThreads, repo));
+		assertEquals("Invalid User setting", 22, getIntVal(getMaximumThreads, repo));
 		System.clearProperty(SimpleArtifactRepository.PROP_MAX_THREADS);
 
 		// Legitimate repo value
 		repo.setProperty(SimpleArtifactRepository.PROP_MAX_THREADS, "4");
 		assertEquals("Valid repository specified setting", 4, getIntVal(getMaximumThreads, repo));
 
+		// Legitimate big repo value, but user default when not specified at all is 4.
+		repo.setProperty(SimpleArtifactRepository.PROP_MAX_THREADS, "8");
+		assertEquals("Valid repository specified setting", 4, getIntVal(getMaximumThreads, repo));
+
 		// User value is lower should take precedence
 		repo.setProperty(SimpleArtifactRepository.PROP_MAX_THREADS, "3");
 		System.setProperty(SimpleArtifactRepository.PROP_MAX_THREADS, "2");
 		assertEquals("User setting should take precedence", 2, getIntVal(getMaximumThreads, repo));
 
-		// User value is lower should take precedence
+		// User value is big but lower should take precedence
+		repo.setProperty(SimpleArtifactRepository.PROP_MAX_THREADS, "10");
+		System.setProperty(SimpleArtifactRepository.PROP_MAX_THREADS, "8");
+		assertEquals("User setting should take precedence", 8, getIntVal(getMaximumThreads, repo));
+
+		// Repo value is lower should take precedence
 		repo.setProperty(SimpleArtifactRepository.PROP_MAX_THREADS, "2");
 		System.setProperty(SimpleArtifactRepository.PROP_MAX_THREADS, "3");
 		assertEquals("User setting should take precedence", 2, getIntVal(getMaximumThreads, repo));
+
+		// Repo value is big but lower should take precedence
+		repo.setProperty(SimpleArtifactRepository.PROP_MAX_THREADS, "8");
+		System.setProperty(SimpleArtifactRepository.PROP_MAX_THREADS, "10");
+		assertEquals("User setting should take precedence", 8, getIntVal(getMaximumThreads, repo));
 	}
 
 	private int getIntVal(Method m, Object repo) throws IllegalArgumentException, IllegalAccessException, InvocationTargetException {