2.15.17 introduces changes in equality

[vendamere]

In 2.15.17 a change was made in PR #2119 that makes two objects no longer equal that were previously equal due to how maps and lists are handled.

Prior to 2.15.17 a and b are equal. (2.15.16)

scala> val a = Message.builder.build()
val a: software.amazon.awssdk.services.sqs.model.Message = Message(Attributes={}, MessageAttributes={})

scala> val b = Message.builder.attributes(new java.util.HashMap()).build()
val b: software.amazon.awssdk.services.sqs.model.Message = Message(Attributes={}, MessageAttributes={})

scala> a == b
val res0: Boolean = true

scala> a.equalsBySdkFields(b)
val res1: Boolean = true

2.15.17 and after a and b are no longer equal (2.15.29)

scala> val a = Message.builder.build()
val a: software.amazon.awssdk.services.sqs.model.Message = Message()

scala> val b = Message.builder.attributes(new java.util.HashMap()).build()
val b: software.amazon.awssdk.services.sqs.model.Message = Message(Attributes={})

scala> a == b
val res0: Boolean = false

scala> a.equalsBySdkFields(b)
val res1: Boolean = false

As far as I know these should still be equivalent. I maintain a library of lenses for AWS using Scala/Monocle and this breaks some of the laws required of the lenses. If this isn't considered a bug I'd like to know so I can decide if I will continue trying to maintain the library.

[millems]

This behavior change was intended. "a" and "b" in your examples aren't technically equal:

> a.hasAttributes()
false
> b.hasAttributes()
true

Is there a reason your use-case requires equals to ignore the hasAttributes method? This behavior for equals is more accurate now, but maybe there's room for a equalsWhereNullAndEmptyAreTheSame (or something more elegantly named) if there's a legitimate use-case we didn't consider.

[vendamere]

This is, I think, probably more directly related to how Scala handles the return of null from the messageAttributes function.

scala> val attributes = Message.builder.build().messageAttributes
val attributes: java.util.Map[String,software.amazon.awssdk.services.sqs.model.MessageAttributeValue] = {}

scala> attributes == null
val res11: Boolean = false

In Scala I can't check if "attributes" is really null or just an empty Map.

Given that I can't obey the identity law of lenses or "set what you get".
For example, if I take get messageAttributes and get an empty Map back I can't update the object with an empty Map and have the same object I started with. Checking for side-effect mutations effectively.

I might be able to use hasAttributes as a proxy for checking if it's really null assuming it's consistent as most of the project is generated by macros though passing null (which is avoided and discouraged in Scala) to some functions wouldn't easily work if there are overloads.

[millems]

It's not just a Scala limitation - it's the same in Java. If you need to see if the response from the service was null or an empty map, the hasAttributes is the correct way of doing it.

We default to empty map even for null map responses from the service, because most of the time the distinction isn't useful, and every service does something a little bit different. Customers kept getting NPEs when we would return null for maps, so we decided to always return an empty map, and if the customer actually cared about the distinction (e.g. union types like DynamoDB's AttributeValue) they could use the has* method.

It's also worth noting that most update* APIs do not require you to pass in the entire "current state" of the resource, just what you want to modify. If you don't specify a value for a specific field, most services will keep that field unmodified.

[vendamere]

I was able to update the code to use has* and pass a null explicitly.

It's not that the entire state of the object must be passed, simply that what you get and what you set are the same and representation shouldn't change by passing back in what you just got. These objects don't technically do that because you get an empty Map and then when you set an empty Map the object's representation changes.

Lenses have to not change the representation in any unintended way to make them compose-able.
The only way I can make my lenses obey that law is to pass in null when I see the update is Empty and the current state of has* is false.

Basically information is lost coming back out of the "get" function. Set(null, EmptyMap, Values) => Set(EmptyMap, Values)
has* lets that information be recovered though.

[millems]

Yeah, I agree that set(null) and then get() returning an empty map is a point of confusion. That's always existed, though, even before the equals method was updated in the PR above. It was a trade-off we made because a lot of customers found it confusing that some services would return null, and some would return empty lists. They would either have to do null checks all of the time or risk getting NPEs. When they asked us what the difference was, we had to admit that "different services do different stuff, so I don't know, just do a null check every time to be safe".

We introduced this confusion because it was less confusing and affected fewer developers than what we used to do: just expose whatever the heck the service wanted to do.

[vendamere]

Thanks for your help. I hadn't really paid attention to the has* functions as check to recover that info. That solves my issue.