Received messages have no event names

[robholmes]

I've proved the message published contains an event name (using the Ably dev console), but in Android/Java MessageListener callback the name field is null. I believe this is because the string equality check is using "reference equality" == rather than "value equality" String.equals().

Examples of this can be found:

• https://github.com/ably/ably-java/blob/master/lib/src/main/java/io/ably/lib/types/Message.java#L85
• https://github.com/ably/ably-java/blob/v1.0.1/lib/src/main/java/io/ably/lib/types/Message.java#L85
• https://github.com/ably/ably-java/blob/v1.1.0-beta.push.1/lib/src/main/java/io/ably/lib/types/Message.java#L108
• https://github.com/ably/ably-java/blob/v1.1.0-beta.push.1/lib/src/main/java/io/ably/lib/types/Message.java#L110

The fix should be to use fieldName.equals("name"), or "name".equals(fieldName) if fieldName could ever be null.

[mattheworiordan]

Thanks @robholmes for alerting us to this issue.. I will get one of the team to investigate tomorrow. @funkyboy can you confirm if you can reproduce this?

[paddybyers]

I'll have a look at this.

I don't think it's quite as simple as you say, because the fieldNames in the lines you highlighted are intern()ed (eg https://github.com/ably/ably-java/blob/v1.1.0-beta.push.1/lib/src/main/java/io/ably/lib/types/Message.java#L103). So reference equality will work when comparing with a literal string.

[robholmes]

@paddybyers I'm not sure that's true since using fieldName == "name" creates a new String of "name" and compares it with the object fieldName, by definition these are different objects. They may have the same value, but the only way to compare a String value is with .equals().

This Gist explains it better than I do: https://gist.github.com/Yengas/9040261#-vs-equals-vs-compareto

[paddybyers]

See https://docs.oracle.com/javase/8/docs/api/java/lang/String.html#intern--

[robholmes]

It is true that I didn't know what the .intern() did until now, but I believe if you really want to use == for a string comparison, then I think you would need to fieldName == "name".intern() (assuming fieldName has already been intern'd). At least that's my understanding, as "name" would not come from the unique intern pool, it's simply a new object.

[paddybyers]

Lets try to reproduce the issue and see what we find.

[robholmes]

Ok thanks Paddy, happy to help where I can. I wasn't able to go as far as writing any tests for this, but I did prove through debugging that the if (fieldName == "name") { was never entered, and it always fell into else {.

[paddybyers]

Ok, that's good information. @funkyboy here will try to reproduce it. There's something unusual about it so let's see.

[funkyboy]

@robholmes I created a sample Android project.
See all the detailed steps in the README file.
I could not reproduce this.

Can you pull the repo and see if you can reproduce it in your setting? And also provide details about your setting? E.g. Java version, etc.

[robholmes]

@funkyboy ok great, I'll give this a try as soon as I can and let you know

[funkyboy]

@robholmes FYI I added you as a collaborator to the repo. Feel free to push your changes in a branch if needed.

[robholmes]

@funkyboy I am currently at a loss to explain it, I cannot reproduce with the sample app, even when I add all our dependencies, gradle plugins, and target/compile versions, etc. I've added the breakpoints in the same place, but it behaves correctly (as expected). I'm using curl also to rule out our message publishing code.

Right now I've hit the brick wall of not knowing what I can try next. I may try cloning the ably-java repo and importing into our project so I can tweak the code and see what I can work out. I'll let you know what I find (if anything).

[funkyboy]

@robholmes Thanks for the update. Let us know if there are new developments.