diff --git a/CHANGELOG.md b/CHANGELOG.md index ab0a1bc07..2e41273c9 100644 --- a/CHANGELOG.md +++ b/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/). diff --git a/modules/bootstrapped/test/src/smithy4s/BlobSpec.scala b/modules/bootstrapped/test/src/smithy4s/BlobSpec.scala index 0522d7636..a320e2639 100644 --- a/modules/bootstrapped/test/src/smithy4s/BlobSpec.scala +++ b/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) diff --git a/modules/core/src/smithy4s/Blob.scala b/modules/core/src/smithy4s/Blob.scala index e2fc758a7..77d090921 100644 --- a/modules/core/src/smithy4s/Blob.scala +++ b/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 {