Enforce DFS blockLimit is a multiple of blockSize

Change-Id: I2821124ff88d7d1812a846ed20f3828fc9123b38

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 45032de9bda483f5b75ab3e89a06819cf073c4dc..3dd72c3cad5d5773f356dc4badc82c0a2e818841 100644 (file)
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -41,6 +41,8 @@ bitmapsMustBePrepared=Bitmaps must be prepared before they may be written.
 blameNotCommittedYet=Not Committed Yet
 blobNotFound=Blob not found: {0}
 blobNotFoundForPath=Blob not found: {0} for path: {1}
+blockLimitNotMultipleOfBlockSize=blockLimit {0} must be a multiple of blockSize {1}
+blockLimitNotPositive=blockLimit must be positive: {0}
 blockSizeNotPowerOf2=blockSize must be a power of 2
 bothRefTargetsMustNotBeNull=both old and new ref targets must not be null.
 branchNameInvalid=Branch name {0} is not allowed

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index e23d2534b6333529508bbeb7ea57b0fa6f84f4ed..0545a4ecafb249993fbd2fda1d3298c5f2a2d41b 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -102,6 +102,8 @@ public class JGitText extends TranslationBundle {
        /***/ public String blameNotCommittedYet;
        /***/ public String blobNotFound;
        /***/ public String blobNotFoundForPath;
+       /***/ public String blockLimitNotMultipleOfBlockSize;
+       /***/ public String blockLimitNotPositive;
        /***/ public String blockSizeNotPowerOf2;
        /***/ public String bothRefTargetsMustNotBeNull;
        /***/ public String branchNameInvalid;

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java
index e558a81c09b5e7d09b73994137371a760e8bbc39..cd7901b357453b22f61f08e242bf8164db9d5176 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java
@@ -95,13 +95,23 @@ public class DfsBlockCacheConfig {
        /**
         * Set maximum number bytes of heap memory to dedicate to caching pack file
         * data.
+        * <p>
+        * It is strongly recommended to set the block limit to be an integer multiple
+        * of the block size. This constraint is not enforced by this method (since
+        * it may be called before {@link #setBlockSize(int)}), but it is enforced by
+        * {@link #fromConfig(Config)}.
         *
         * @param newLimit
         *            maximum number bytes of heap memory to dedicate to caching
-        *            pack file data.
+        *            pack file data; must be positive.
         * @return {@code this}
         */
        public DfsBlockCacheConfig setBlockLimit(final long newLimit) {
+               if (newLimit <= 0) {
+                       throw new IllegalArgumentException(MessageFormat.format(
+                                       JGitText.get().blockLimitNotPositive,
+                                       Long.valueOf(newLimit)));
+               }
                blockLimit = newLimit;
                return this;
        }
@@ -188,23 +198,34 @@ public class DfsBlockCacheConfig {
         * <p>
         * If a property is not defined in the configuration, then it is left
         * unmodified.
+        * <p>
+        * Enforces certain constraints on the combination of settings in the config,
+        * for example that the block limit is a multiple of the block size.
         *
         * @param rc
         *            configuration to read properties from.
         * @return {@code this}
         */
        public DfsBlockCacheConfig fromConfig(final Config rc) {
-               setBlockLimit(rc.getLong(
+               long cfgBlockLimit = rc.getLong(
                                CONFIG_CORE_SECTION,
                                CONFIG_DFS_SECTION,
                                CONFIG_KEY_BLOCK_LIMIT,
-                               getBlockLimit()));
-
-               setBlockSize(rc.getInt(
+                               getBlockLimit());
+               int cfgBlockSize = rc.getInt(
                                CONFIG_CORE_SECTION,
                                CONFIG_DFS_SECTION,
                                CONFIG_KEY_BLOCK_SIZE,
-                               getBlockSize()));
+                               getBlockSize());
+               if (cfgBlockLimit % cfgBlockSize != 0) {
+                       throw new IllegalArgumentException(MessageFormat.format(
+                                       JGitText.get().blockLimitNotMultipleOfBlockSize,
+                                       Long.valueOf(cfgBlockLimit),
+                                       Long.valueOf(cfgBlockSize)));
+               }
+
+               setBlockLimit(cfgBlockLimit);
+               setBlockSize(cfgBlockSize);
 
                setConcurrencyLevel(rc.getInt(
                                CONFIG_CORE_SECTION,