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

Extra comma in json results when null column is the first one processed #25

Closed
mooreds opened this Issue Oct 12, 2012 · 0 comments

Comments

Projects
None yet
2 participants
@mooreds
Copy link
Contributor

mooreds commented Oct 12, 2012

Hi,

I'm running into an issue when I output json and some of the fields in the select statement are null.

If any of those fields happen to be the first one, we get an extra comma in the output, and that means the result is not valid json.

To see an example of this, make the following modifications to the sakila database:

alter table language add column mynullfield varchar(25);
alter table language add column anullfield varchar(25);
alter table language add column bnullfield varchar(25);
alter table language add column cnullfield varchar(25);
alter table language add column dnullfield varchar(25);
alter table language add column enullfield varchar(25);
update language set bnullfield = "b", anullfield = "a" where language_id = 2;

Update LangFilm.xml to be this:

<?xml version="1.0" encoding="UTF-8"?>
<rs:sqlResource xmlns:rs="http://restsql.org/schema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:schemaLocation="http://restsql.org/schema SqlResource.xsd ">
        <query>
                select anullfield,bnullfield,cnullfield,dnullfield,name
                from language
        </query>
        <metadata>
                <database default="sakila" />
                <table name="language" alias="lang" role="Parent" />
        </metadata>
</rs:sqlResource>

Then, if you visit that resource:
[restsql path]/res/lang.LangFilm?_limit=10&_offset=0&_output=json

you should see something like this:

{ "langs": [
        { , "name": "English" },
        { "anullfield": "a", "bnullfield": "b", "name": "Italian" },
        { , "name": "Japanese" },
        { , "name": "Mandarin" },
        { , "name": "French" },
        { , "name": "German" }
    ]
}

That doesn't validate at jsonlint.org.

If you don't see this, column display order may be platform dependent, but if you remove the name field, you should definitely see the prepended commas.

The issue appears to be in org.restsql.core.impl.JsonResponseSerializer.appendNameValuePair. If the first value is null, then we don't display it--all well and good. But that means that the firstPair boolean is false for subsequent calls, and that causes the errant comma.

Proposed solution--use a json generation library: http://stackoverflow.com/questions/1668862/good-json-java-library rather than a hand crafted solution. Alternative solution: revamp the org.restsql.core.impl.JsonResponseSerializer.appendNameValuePair method to handle this case. Here's a test case that fails (you have to make appendNameValuePair package visible rather than private). The case that fails is testAppendNameValuePairNullFirst

package org.restsql.core.impl;

import static org.junit.Assert.*;

import org.junit.Test;

public class JsonResponseSerializerTest {

    @Test
    public void testAppendNameValuePairNormal() {
        JsonResponseSerializer jrs = new JsonResponseSerializer();
        assertNotNull(jrs);
        StringBuilder sb = new StringBuilder();
        jrs.appendNameValuePair(true, sb, "bar", "baz");

        jrs.appendNameValuePair(false, sb, "bar2", "baz2");

        assertEquals("\"bar\": \"baz\", \"bar2\": \"baz2\"",sb.toString());
    }

    @Test
    public void testAppendNameValuePairOne() {
        JsonResponseSerializer jrs = new JsonResponseSerializer();
        assertNotNull(jrs);
        StringBuilder sb = new StringBuilder();
        jrs.appendNameValuePair(true, sb, "bar", "baz");

        assertEquals("\"bar\": \"baz\"",sb.toString());
    }

    @Test
    public void testAppendNameValuePairOneNull() {
        JsonResponseSerializer jrs = new JsonResponseSerializer();
        assertNotNull(jrs);
        StringBuilder sb = new StringBuilder();
        jrs.appendNameValuePair(true, sb, "bar", null);

        assertEquals("",sb.toString());
    }

    @Test
    public void testAppendNameValuePairNone() {
        JsonResponseSerializer jrs = new JsonResponseSerializer();
        assertNotNull(jrs);
        StringBuilder sb = new StringBuilder();

        assertEquals("",sb.toString());
    }



    @Test
    public void testAppendNameValuePairNullFirst() {
        JsonResponseSerializer jrs = new JsonResponseSerializer();
        assertNotNull(jrs);
        StringBuilder sb = new StringBuilder();
        jrs.appendNameValuePair(true, sb, "bar", null);

        jrs.appendNameValuePair(false, sb, "bar2", "baz2");

        assertEquals("\"bar2\": \"baz2\"",sb.toString());
    }

    @Test
    public void testAppendNameValuePairNullInMiddle() {
        JsonResponseSerializer jrs = new JsonResponseSerializer();
        assertNotNull(jrs);
        StringBuilder sb = new StringBuilder();
        jrs.appendNameValuePair(true, sb, "bar", "baz");

        jrs.appendNameValuePair(false, sb, "bar3", null);

        jrs.appendNameValuePair(false, sb, "bar2", "baz2");

        assertEquals("\"bar\": \"baz\", \"bar2\": \"baz2\"",sb.toString());
    }

    @Test
    public void testAppendNameValuePairNullAtEnd() {
        JsonResponseSerializer jrs = new JsonResponseSerializer();
        assertNotNull(jrs);
        StringBuilder sb = new StringBuilder();
        jrs.appendNameValuePair(true, sb, "bar", "baz");

        jrs.appendNameValuePair(false, sb, "bar2", "baz2");

        jrs.appendNameValuePair(false, sb, "bar3", null);

        assertEquals("\"bar\": \"baz\", \"bar2\": \"baz2\"",sb.toString());

    }


}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment