Merge pull request #951 from richarda23/crc64checksumtest

CRC64Checksum test #944

CHANGELOG.md

@@ -31,6 +31,8 @@ BioJava 6.0.0 (future release)
 * Moved all chem-comp model classes from `org.biojava.nbio.structure.io.mmcif.chem` to `org.biojava.nbio.structure.chem`
 * Moved all chem-comp parsing classes from `org.biojava.nbio.structure.io.mmcif.chem` to `org.biojava.nbio.structure.io.cif`
 * Moved classes in `org.biojava.nbio.structure.io.mmcif` to `org.biojava.nbio.structure.chem`
+* Fixed `CRC64Checksum#public void update(byte[] b, int offset, int length)` to use
+the `length` argument correctly as specified in `java.util.zip.Checksum` interface.
 
 ### Fixed
 * Correct chain assignment to entities when parsing PDB/mmCIF without entity information (in cases with more than 3 chains per entity) #931

biojava-core/src/main/java/org/biojava/nbio/core/util/CRC64Checksum.java

@@ -56,9 +56,27 @@ public void update(int b) {
 		crc = low ^ high;
 	}
 
+	 /**
+     * Updates the CRC-64 checksum with the specified array of bytes.
+	 * <br/>
+	 * Note that BioJava before version 6.0 implemented this method incorrectly,
+	 * using {@code length} as an index.
+     *
+     * @throws IllegalArgumentException
+     *         if {@code offset} is negative, or {@code length} is negative, or
+     *         {@code offset+length} is negative or greater than the length of
+     *         the array {@code b}.
+     */
 	@Override
 	public void update(byte[] b, int offset, int length) {
-		for (int i = offset; i < length; ++i)
+		if (b == null) {
+            throw new IllegalArgumentException("byte array cannot be null");
+        }
+        if (offset < 0 || length < 0 || offset > b.length - length) {
+            throw new IllegalArgumentException("Offset and length must be non-negative"+
+			 " and their sum cannot be greater than length of byte array");
+        }
+		for (int i = offset; i < length + offset; ++i)
 			update(b[i]);
 	}
 

biojava-core/src/test/java/org/biojava/nbio/core/util/CRC64ChecksumTest.java

@@ -0,0 +1,109 @@
+package org.biojava.nbio.core.util;
+
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.RepeatedTest;
+import org.junit.jupiter.api.Test;
+/** 
+*  
+* @author Richard Adams
+*/
+class CRC64ChecksumTest {
+    CRC64Checksum crc64 = null;
+    private final String helloInCrc64Hex = "53C1111D27800000";
+    private final Long helloInCrc64Decimal = 6035123792567599104L;
+
+    @BeforeEach
+    void before (){
+         crc64 = new CRC64Checksum();
+    }
+
+    @Test
+    @DisplayName("Default value is 0")
+    void initialBehaviour() {
+        assertEquals(0, crc64.getValue());
+        assertEquals("0000000000000000", crc64.toString());
+    }
+
+    @RepeatedTest(10)
+    void sameInputRepeatedlyGeneratesSameOutput(){
+        crc64.update("hello");
+        assertEquals(helloInCrc64Decimal, crc64.getValue());
+        assertEquals(helloInCrc64Hex, crc64.toString());
+    }
+
+    @Test
+    void afterResettingCrcIsZero(){
+        crc64.update("hello");
+        crc64.reset();
+        assertEquals(0, crc64.getValue());
+    }
+
+    @Test
+    void addingIncrementallyIsSameAsAllAtOnce(){
+        crc64.update("h");
+        crc64.update("e");
+        crc64.update("l");
+        crc64.update("l");
+        crc64.update("o");
+        assertEquals(helloInCrc64Hex, crc64.toString());
+    }
+
+    @Test
+    void allbyteRange (){
+        byte [] testBytes = new byte [] {1,2,3,4,5};
+        crc64.update(testBytes, 0, testBytes.length);
+        String allBytesHex = crc64.toString();
+        crc64.reset();
+        for (byte b: testBytes) {
+            crc64.update(b);
+        }
+        assertEquals(allBytesHex, crc64.toString());
+    }
+
+    @Test
+    void allRangeIsSameAsAllArray (){
+        byte [] testBytes = new byte [] {1,2,3,4,5};
+        crc64.update(testBytes, 0, testBytes.length);
+        Long valueFromAllRange = crc64.getValue();
+        crc64.reset();
+        crc64.update(testBytes);
+        assertEquals(valueFromAllRange, crc64.getValue());
+    }
+
+    @Test 
+    void partialByteRange (){
+        byte [] testBytes = new byte [] {1,2,3,4,5};
+        crc64.update(testBytes, 2, 1);
+        String partialBytesHex = crc64.toString();
+        crc64.reset();
+        crc64.update(testBytes[2]);
+        assertEquals(partialBytesHex, crc64.toString());
+    }
+
+    @Test 
+    void partialByteRangeRejectsInvalidInput (){
+        byte [] testBytes = new byte [] {1,2,3,4,5};
+        assertAll(
+            ()->assertThrows(IllegalArgumentException.class,
+                ()->crc64.update(testBytes, -1, 0)),
+            ()->assertThrows(IllegalArgumentException.class,
+                ()->crc64.update(testBytes, 0, -1)),
+            ()->assertThrows(IllegalArgumentException.class,
+                ()->crc64.update(testBytes, 0, testBytes.length+1)),
+            ()->assertThrows(IllegalArgumentException.class,
+                ()->crc64.update(testBytes,  testBytes.length, 1))
+        );
+    }
+
+
+    @Test
+    void hexStringIsEqualToValue(){
+        Long value = Long.parseLong(helloInCrc64Hex, 16);
+        assertEquals(helloInCrc64Decimal, value);
+    }
+}