zio-http icon indicating copy to clipboard operation
zio-http copied to clipboard

Delegate equals and hashCode of zhttp.service.Channel to JChannel

Open mhodovaniuk opened this issue 1 year ago • 6 comments

Describe the bug zhttp.service.Channel has deafault equals and hashCode. As a result, two zhttp.service.Channel are not equal even though they have the same channel: JChannel field inside.

Expected behaviour equals and hashCode in zhttp.service.Channel should be defined and implementation should be - delegate to channel: JChannel field's equals and hashCode. Two zhttp.service.Channel are equal if channel: JChannel fields are equal no matter what convert function is.

mhodovaniuk avatar Aug 11 '22 15:08 mhodovaniuk

Thats right. Thanks for reporting 🙏, would you be interested in creating a fix for this🙂

tusharmath avatar Aug 12 '22 04:08 tusharmath

I thought about this a bit again —

def c1: Channel[String] = ???
def c2: Channel[Array[Byte]] = c1.contramap(_.getBytes)

We can't say that c1 == c2 because they both take in different type of inputs. But with the proposed changes, it's going to be equal.

I would want to understand more about the exact use-case.

tusharmath avatar Aug 12 '22 08:08 tusharmath

The use case is to maintain a collection of active subscribers: e.g. have some Map[Channel[A], V]. The point is, that from a library client's point of view channels are equal if underlying JChannels are the same.

Currently, equals is broken: convert functions are always different

val jChannel1: EmbeddedChannel = new EmbeddedChannel()
val c1                         = Channel[String](jChannel1, _.getBytes)
val c2                         = Channel[String](jChannel1, _.getBytes)
assertTrue(c1 == c2) // fails:
    ✗ Channel(
          channel = [id: 0xembedded, L:embedded - R:embedded],
          convert = zhttp.service.ChannelSpec$$$Lambda$1001/0x0000000800f45320@72dafb6
        ) was not equal to Channel(
          channel = [id: 0xembedded, L:embedded - R:embedded],
          convert = zhttp.service.ChannelSpec$$$Lambda$1007/0x0000000800f46608@a4dce17
        )

mhodovaniuk avatar Aug 12 '22 09:08 mhodovaniuk

What if we expose id on channel instead?

final case class Channel[-A](private val channel: JChannel,  private val convert: A => Any) { self =>
  def id: UIO[String] = ZIO.succeed(channel.id().asLongText())

  ...
}

tusharmath avatar Aug 12 '22 09:08 tusharmath

Actually the method id already exists.

tusharmath avatar Aug 12 '22 09:08 tusharmath

Yes, the workaround with id works and this is not a blocker. And that is exactly how I fixed this in my code.

The intention of this ticket is to improve API and make behavior more expectable, since the class name is Channel and it is a wrapper around the Netty Channel, and the user expects it to have behavior similar to the Netty Channel. Also, I am not sure about the use case where current equals works.

I hope this all makes sense. I really do not want to fix my exact use case, but improve the usability of the library😊

mhodovaniuk avatar Aug 12 '22 10:08 mhodovaniuk

I think in that case may be we shouldn't have Channel as a case class.

tusharmath avatar Aug 17 '22 08:08 tusharmath

Closing this issue for now. We can come to this again, when we have a better solution.

tusharmath avatar Aug 25 '22 06:08 tusharmath