Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON output : not returning empty string but empty json array instead #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ghetolay
Copy link

On QidoRS when there is no match for a request a empty body is returned which is a problem when client expect a JSON result.
An empty string is invalid JSON and breaks on most system, returning an empty array instead '[]' is correct JSON and is less error prone.

From json.org :

JSON is built on two structures:

- A collection of name/value pairs. In various languages, this is realized as an object, record, struct, dictionary, hash table, keyed list, or associative array.
- An ordered list of values. In most languages, this is realized as an array, vector, list, or sequence.

So a json message can't be just a string (empty or not) it can only be an object {...} or array [...]
( some system allows nullas well).

Example of system rejecting empty string as JSON :

  • Javascript function JSON.parse() consider an empty string as invalid JSON and throws error
  • Java JsonParser (glassfish implementation) also throws an exception with the message :
    Expected tokens are: [CURLYOPEN, SQUAREOPEN]
    (Didn't test myself but qido-client is also most likely affected by this)

Now even if some implementation accept empty string as valid JSON it's safer to return an empty array.

About code change :

For XML output I kept the return of empty string by returning a null entity when no match are found (javax.ws.rs.core.Response.ResponseBuilder.entity(Object)accepts null as argument).

An empty string is invalid JSON and break on most system, returning an empty array instead '[]' is correct JSON and is less error prone.
@ghetolay
Copy link
Author

Ho ok didn't know there was an active jira. What's the url ? The one mentioned on the readme brings to a project with no issue so I thought this was not used.

Next time I'll look into it before.

About the implementation I don't know what you exactly mean by tackle but I think writeXML is the real problem here. Problem which was covered with theses lines on the search function :

if (!query.hasMoreMatches())
    return Response.ok().build();

It seems more logical to me that output.entity() returns the correct entity (null, empty, text, multipart etc..) rather than bypassing the call on some condition. And writeXML(), as it is, returns a multipart entity when there is no match rather than no entity.

What I don't like on my fix is the use of count which is a variable used for debug logging purpose (realized that later). So maybe a global if would be better like :

private Object writeXML(Query query, QueryRetrieveLevel qrlevel) {
    if(query.hasMoreMatches())
        MultipartRelatedOutput output = new MultipartRelatedOutput();
        int count = 0;
        while (query.hasMoreMatches()) {
            ....
       }
        return output;
    } else 
        return null;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants