Merge pull request #1526 from msosnicki/blob-equals

Change Blob equals semantics + fix tests

CHANGELOG.md

@@ -5,6 +5,10 @@ When adding entries, please treat them as if they could end up in a release any
 
 Thank you!
 
+# 0.18.20 
+
+* Change semantics of `Blob.equals` - Blobs do not take underlying type into consideration, just bytes in https://github.com/disneystreaming/smithy4s/pull/1526
+
 # 0.18.19 - binary-breaking changes in `core`
 
 **WARNING**: This release includes binary-breaking changes in the `core` module. This is indirectly caused by an upstream change in [smithy-lang/smithy](https://github.com/smithy-lang/smithy/).

modules/bootstrapped/test/src/smithy4s/BlobSpec.scala

@@ -21,15 +21,37 @@ import munit._
 import java.nio.ByteBuffer
 import java.io.ByteArrayOutputStream
 import scala.util.Using
-class BlobSpec() extends FunSuite {
+import org.scalacheck.Prop._
+import scala.collection.immutable.Queue
+class BlobSpec() extends ScalaCheckSuite {
+
+  property("equals and hashcode contract") {
+    forAll { (value: String) =>
+      val bytes = value.getBytes()
+      val array = Blob(value)
+      val byteBuffer = Blob(ByteBuffer.wrap(bytes))
+      val queue = Blob.queue(
+        Queue(bytes.map(b => Blob(ByteBuffer.wrap(Array(b)))): _*),
+        bytes.size
+      )
+
+      assert(array == byteBuffer)
+      assert(array == queue)
+      assert(array.hashCode() == byteBuffer.hashCode())
+      assert(array.hashCode() == queue.hashCode())
+    }
+  }
 
   test("sameBytesAs works across data structures") {
     assert(Blob("foo").sameBytesAs(Blob("foo".getBytes)))
     assert(Blob("foo").sameBytesAs(Blob(ByteBuffer.wrap("foo".getBytes))))
   }
 
-  test("equals depends on underlying data structure") {
-    assert(Blob("foo") != Blob(ByteBuffer.wrap("foo".getBytes)))
+  test("equals does not depend on underlying data structure") {
+    assert(Blob("foo") == Blob(ByteBuffer.wrap("foo".getBytes)))
+    assert(
+      Blob("foo") == Blob(ByteBuffer.wrap("f".getBytes)).concat(Blob("oo"))
+    )
     assert(Blob("foo") == Blob("foo"))
     assert(
       Blob(ByteBuffer.wrap("foo".getBytes)) == Blob(
@@ -56,6 +78,16 @@ class BlobSpec() extends FunSuite {
     assertNotEquals(blob1.hashCode, blob3.hashCode)
   }
 
+  test("QueueBlob.hashcode is consistent") {
+    def makeBlob(str: String) =
+      Blob(str.getBytes).concat(Blob(ByteBuffer.wrap(str.getBytes())))
+    val blob1 = makeBlob("foo")
+    val blob2 = makeBlob("foo")
+    val blob3 = makeBlob("bar")
+    assertEquals(blob1.hashCode, blob2.hashCode)
+    assertNotEquals(blob1.hashCode, blob3.hashCode)
+  }
+
   test("Concat works as expected") {
     val blob = Blob("foo") ++ Blob("bar")
     assertEquals(blob.size, 6)

modules/core/src/smithy4s/Blob.scala

@@ -113,6 +113,20 @@ sealed trait Blob {
       }
 
   final def ++(other: Blob) = concat(other)
+
+  override def equals(other: Any): Boolean =
+    other match {
+      case otherBlob: Blob => sameBytesAs(otherBlob)
+      case _               => false
+    }
+
+  override def hashCode(): Int = {
+    import util.hashing.MurmurHash3
+    var h = MurmurHash3.stringHash("Blob")
+    foreach(o => h = MurmurHash3.mix(h, o.##))
+    MurmurHash3.finalizeHash(h, size)
+  }
+
 }
 
 object Blob {
@@ -171,12 +185,7 @@ object Blob {
     override def toString = s"ByteBufferBlob(...)"
     override def isEmpty: Boolean = !buf.hasRemaining()
     override def size: Int = buf.remaining()
-    override def hashCode = buf.hashCode()
 
-    override def equals(other: Any): Boolean = {
-      other.isInstanceOf[ByteBufferBlob] &&
-      buf.compareTo(other.asInstanceOf[ByteBufferBlob].buf) == 0
-    }
   }
 
   final class ArraySliceBlob private[smithy4s] (val arr: Array[Byte], val offset: Int, val length: Int) extends Blob {
@@ -206,23 +215,6 @@ object Blob {
 
     override def toString(): String = s"ArraySliceBlob(..., $offset, $length)"
 
-    override def hashCode(): Int = {
-      import util.hashing.MurmurHash3
-      var h = MurmurHash3.stringHash("ArraySliceBlob")
-      h = MurmurHash3.mix(h, MurmurHash3.arrayHash(arr))
-      h = MurmurHash3.mix(h, offset)
-      MurmurHash3.mixLast(h, length)
-    }
-
-    override def equals(other: Any): Boolean = {
-      other.isInstanceOf[ArraySliceBlob] && {
-        val o = other.asInstanceOf[ArraySliceBlob]
-        offset == o.offset &&
-        length == o.length &&
-        java.util.Arrays.equals(arr, o.arr)
-      }
-    }
-
   }
 
   final class QueueBlob private[smithy4s] (val blobs: Queue[Blob], val size: Int) extends Blob {