delight-im / Android-DDP

[UNMAINTAINED] Meteor's Distributed Data Protocol (DDP) for clients on Android
Apache License 2.0
274 stars 54 forks source link

String with two double quotes in RequestListener #71

Open daupawar opened 8 years ago

daupawar commented 8 years ago

This is my code

("myMethodName", new Object[]{values}, new ResultListener() {
                    @Override
                    public void onSuccess(String result) {

                  }
}

at onSuccess i am getting result with two double quotes ""resultValue"";

then my following code return me false because of two double quotes

 if (result.equalsIgnoreCase("resultValue")) {
    return true;
}else{
    return false;
}

on server

myMethodName: function () {
    return 'resultValue';
  },
ocram commented 8 years ago

Thanks for this great report!

What you receive in your app should be "resultValue" instead of resultValue, right?

The ""resultValue""; that you wrote is a bit confusing. Maybe you wanted to say that, as a Java string, it would be String str = "\"resultValue\""; instead of String str = "resultValue";? Anyway, the difference described in the previous paragraph is more important. Is that what you see?

What is returned as the result is JavaScript data. Here's the current situation:

Sent from JavaScript (server) Received in Java (client)
return "hello world"; "hello world"
return 203948; 203948
return {"name":"John Doe","age":85}; {"name":"John Doe","age":85}
return JSON.stringify({"name":"John Doe","age":85}); "{\"name\":\"John Doe\",\"age\":85}"

So, as you can see, this works perfectly. What you receive is valid JavaScript and can be parsed as JSON, for example. This allows you to receive complex values in a JSON object, by the way.

But if you expect plain strings as your response, that doesn't work, of course.

So if we changed this line from toString to getTextValue, here's what would happen:

Sent from JavaScript (server) Received in Java (client)
return "hello world"; hello world
return 203948; null
return {"name":"John Doe","age":85}; null
return JSON.stringify({"name":"John Doe","age":85}); {"name":"John Doe","age":85}

The problem is, whoever relies on some of these behaviors, will have broken code if we change this. And the internal methods rely on this behavior as well. Thus they would have to be changed, too.

Don't know what to do. Maybe the best thing we could do is to keep the current behavior. Then, whenever we detect that the result both begins with " and ends with ", we convert it to a plain string (i.e. the part in between). What do you think?

Maybe we might replace that one line with the following lines to solve the problem:

if (data.get(Protocol.Field.RESULT).isTextual()) {
    result = data.get(Protocol.Field.RESULT).getTextValue();
}
else {
    result = data.get(Protocol.Field.RESULT).toString();
}

Anyway, nothing of all this is documented. So, great question!

daupawar commented 8 years ago

Hello,

Thanks for your great answer and explanation.

please check following images result while debug mode in Android studio Image 1:- screen shot 2016-03-14 at 15 57 40

result in debug watch in Android studio Image 2:- screen shot 2016-03-14 at 15 57 58

you can see two double quotes before and after my result

changing current behavior in library would be problem for many developers. so i solved my problem in this way

@Override
public void onSuccess(String result) {
  //my result is ""resultCode"" like image 1 and 2
  result=result.replaceAll("^\"|\"$", "");
  //now result is "resultCode"
}

result after above code Image 3:- screen shot 2016-03-14 at 15 58 34

in debug watch screen shot 2016-03-14 at 15 58 47

ocram commented 8 years ago

Thank you very much!

That replaceAll call using a regular expression seems like a perfect (temporary) solution. Thanks for sharing, and glad to hear that it's working for you now.

In the next major version, we could change the signatur of the ResultListener callbacks from ...

void onSuccess(String result)

... to ...

void onSuccess(String result, boolean isJson)

... or even ...

void onSuccess(String result, boolean isJson, boolean isText, boolean isNumber)

What do you think?

This way, we could parse the text (and remove the extra double quotes). To prevent things from breaking (silently) for some users, these new parameters would be added. So when isText is true, you know that you're receiving plain text (not JSON) and you don't have to remove the double quotes yourself anymore.

daupawar commented 8 years ago

Changin signatur of ResultListner would be great idea, but every time while handling "onSuccess" we need to check type of result like this

       if(isJson){
           //do something with result
        } 
         else if(isText){
             //do something with result
         }
          else if(isNumber){
               //do something with result 
            }

this might be a extra code to do every time

ocram commented 8 years ago

@daupawar Usually, you know the format already, right? You define it on the server, so you actually know whether a plain string or a JSON object is expected.

Thus you could use these new parameters as optional help only.

We should definitely change something about the plain string issues that you discovered. In your case, the simple regular expression works. But there are more complex cases:

If you expect {"name":"John Doe","age":85}, you actually get "{\"name\":\"John Doe\",\"age\":85}" if we don't change anything. And as a user of this library, you really shouldn't have to unescape this.

We should simply this process. Do you agree?

daupawar commented 8 years ago

Yes, i totally agree with you. void onSuccess(String result, boolean isJson, boolean isText, boolean isNumber) is good solution for all