DanielaSfregola / twitter4s

An asynchronous non-blocking Scala client for both the Twitter Rest and Streaming API
Apache License 2.0
256 stars 101 forks source link

Fixing twitter profile image blank issue #456

Closed thiago-preira closed 2 years ago

thiago-preira commented 2 years ago

Withheld tweets does not contain a proper profile image url, as API response extracted below:

{
  "created_at":"Thu Mar 03 09:53:12 +0000 2022",
  "id":1499321986073432067,
  "id_str":"1499321986073432067",
  "text":"@sputnik_ar's account has been withheld in Belgium, Austria, Bulgaria, Sweden, Croatia, Spain, Slovenia, Cyprus, Slovakia, Czech Republic, Romania, Portugal, Poland, Denmark, Netherlands, Estonia, Malta, Luxembourg, Finland, France, Lithuania, Germany, Greece, Latvia, Hungary, Italy, Ireland in response to a legal demand. Learn more.",
...
"profile_image_url":"",
"profile_image_url_https":"",
...
}

This causes an java.lang.StringIndexOutOfBoundsException: String index out of range: -1 at ProfileImage.scala line 18 To fix the issue I'm just forwarding the empty url for images

DanielaSfregola commented 2 years ago

Hi Thiago! thanks for raising this :)

Have you tried making the profile image optional in the case class? It may be easier (and more elegant) to fix that way as the serialization/deserialization will be handled for you without defining an empty ProfileImage.

Cheers, Daniela

thiago-preira commented 2 years ago

Hi Daniela, thanks for your reply. As mentioned above, the twitter API sends the profile_image_url and profile_image_url_https fields as empty string rather than null, that's why I have implemented the fix that way. Please let me know if there is anything that you think it should be updated. Cheers, Thiago

DanielaSfregola commented 2 years ago

Hi @thiago-preira, we fully control how we serialize/deserialize from Twitter, so I propose we do the following:

private[twitter4s] case object ProfileImageSerializer
    extends CustomSerializer[ProfileImage](format =>
      ({
        case JString("") => null // we consider returning an empty string equivalent to null
        case JString(n) => ProfileImage(n)
        case JNull      => null
      }, {
        case img: ProfileImage => JString(img.normal)
      }))

from CustomFormats.scala

And then we change ProfileImage to be an Option[ProfileImage] in the case classes where this is used. This is indeed an API change (we'll need a version bump!), but it better represents that a profile image is not available for the user.

WDYT?

Cheers, Daniela

thiago-preira commented 2 years ago

Makes total sense! Sorry, wasn't aware of custom serialization/deserialization. Definitely way more elegant solution. Not sure about the version bump. I have increased to 8.0 I have applied the fixes, please let me know if any other adjustments are needed. Cheers, Thiago