Apache Jena
  1. Apache Jena
  2. JENA-184

RDFJSONWriter writes illegal JSON when there is two objects of the same subject-predicate.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: ARQ 2.9.1
    • Component/s: RIOT
    • Labels:
      None

      Description

      RDFJSONWriter writes illegal JSON when there is two objects of the same subject-predicate.

      But the RDF/JSON reader can read the illegal JSON.

      1. Jena184_RDFJSONWriter.java
        3 kB
        Andy Seaborne
      2. RdfJsonBadObjectListPatch.patch
        3 kB
        Rob Vesse
      3. RdfJsonBadObjectListArrayWritingPatch.patch
        1 kB
        Rob Vesse
      4. invalid.json
        0.3 kB
        Rurik Thomas Greenall
      5. sparqlProcessor.txt
        0.5 kB
        Rurik Thomas Greenall
      6. JSONTest.txt
        2 kB
        Rurik Thomas Greenall
      7. JENA-184-write-read-tests.txt
        4 kB
        Andy Seaborne

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Jena_ARQ #416 (See https://builds.apache.org/job/Jena_ARQ/416/)
          JENA-184

          Additional tests, thank you Andy.

          castagna :
          Files :

          • /incubator/jena/Jena2/ARQ/trunk/src/test/java/org/openjena/riot/lang/TestLangRdfJson.java
          • /incubator/jena/Jena2/ARQ/trunk/src/test/java/org/openjena/riot/out/TestOutputRDFJSON.java
          Show
          Hudson added a comment - Integrated in Jena_ARQ #416 (See https://builds.apache.org/job/Jena_ARQ/416/ ) JENA-184 Additional tests, thank you Andy. castagna : Files : /incubator/jena/Jena2/ARQ/trunk/src/test/java/org/openjena/riot/lang/TestLangRdfJson.java /incubator/jena/Jena2/ARQ/trunk/src/test/java/org/openjena/riot/out/TestOutputRDFJSON.java
          Hide
          Andy Seaborne added a comment -

          With these additional tests, can we now close this JIRA?

          Show
          Andy Seaborne added a comment - With these additional tests, can we now close this JIRA?
          Hide
          Andy Seaborne added a comment -

          More tests of the write-read cycle of RDF/JSON.

          Show
          Andy Seaborne added a comment - More tests of the write-read cycle of RDF/JSON.
          Hide
          Hudson added a comment -

          Integrated in Jena_ARQ #405 (See https://builds.apache.org/job/Jena_ARQ/405/)
          JENA-184

          Let's parse the JSON back to make sure we always produce valid JSON.

          castagna :
          Files :

          • /incubator/jena/Jena2/ARQ/trunk/src/test/java/org/openjena/riot/lang/UnitTestRDFJSON.java
          Show
          Hudson added a comment - Integrated in Jena_ARQ #405 (See https://builds.apache.org/job/Jena_ARQ/405/ ) JENA-184 Let's parse the JSON back to make sure we always produce valid JSON. castagna : Files : /incubator/jena/Jena2/ARQ/trunk/src/test/java/org/openjena/riot/lang/UnitTestRDFJSON.java
          Hide
          Paolo Castagna added a comment -

          Rurik you can find a new ARQ SNAPSHOT in the usual place:
          https://repository.apache.org/content/repositories/snapshots/org/apache/jena/jena-arq/2.9.1-incubating-SNAPSHOT/
          Please, add a comment here if you still have problems.
          Thanks Andy, thanks Rob.

          Show
          Paolo Castagna added a comment - Rurik you can find a new ARQ SNAPSHOT in the usual place: https://repository.apache.org/content/repositories/snapshots/org/apache/jena/jena-arq/2.9.1-incubating-SNAPSHOT/ Please, add a comment here if you still have problems. Thanks Andy, thanks Rob.
          Hide
          Hudson added a comment -

          Integrated in Jena_ARQ #404 (See https://builds.apache.org/job/Jena_ARQ/404/)
          JENA-184

          More test cases should be added to TestOutputRDFJSON (I am looking what data already in ARQ I could use for that).

          castagna :
          Files :

          • /incubator/jena/Jena2/ARQ/trunk/src/main/java/org/openjena/atlas/json/io/JSWriter.java
          • /incubator/jena/Jena2/ARQ/trunk/src/main/java/org/openjena/riot/lang/LangRDFJSON.java
          • /incubator/jena/Jena2/ARQ/trunk/src/main/java/org/openjena/riot/out/SinkEntityOutput.java
          • /incubator/jena/Jena2/ARQ/trunk/src/test/java/org/openjena/riot/out/TestOutputRDFJSON.java
          Show
          Hudson added a comment - Integrated in Jena_ARQ #404 (See https://builds.apache.org/job/Jena_ARQ/404/ ) JENA-184 More test cases should be added to TestOutputRDFJSON (I am looking what data already in ARQ I could use for that). castagna : Files : /incubator/jena/Jena2/ARQ/trunk/src/main/java/org/openjena/atlas/json/io/JSWriter.java /incubator/jena/Jena2/ARQ/trunk/src/main/java/org/openjena/riot/lang/LangRDFJSON.java /incubator/jena/Jena2/ARQ/trunk/src/main/java/org/openjena/riot/out/SinkEntityOutput.java /incubator/jena/Jena2/ARQ/trunk/src/test/java/org/openjena/riot/out/TestOutputRDFJSON.java
          Hide
          Rurik Thomas Greenall added a comment -

          Andy, — This is related to a local problem in my setup; I ran the attached code and now get the expected result.

          Please do apply the patch.

          Show
          Rurik Thomas Greenall added a comment - Andy, — This is related to a local problem in my setup; I ran the attached code and now get the expected result. Please do apply the patch.
          Hide
          Andy Seaborne added a comment -

          I have applied the two patches to the ARQ development codebase, run the example from sparqlProcessor.txt as best I can and it works for me (I get legal JSON output).

          I used a query of

          DESCRIBE <http://www.w3.org/2004/02/skos/core#prefLabel>

          and

          DESCRIBE <http://api.talis.com/stores/mesh-norwegian/items/D016315>

          both of which have several skos:alkLabels.

          Rurik - please provide an complete minimal example (ideally not using Talis's remote service as this is unrelated to the RDFJSON out on a local graph). Extracts from other system that aren't standalone make it hard to know if some other factor is involved for example other handling of the HttpServletResponse.

          I propose we apply these patches.

          Show
          Andy Seaborne added a comment - I have applied the two patches to the ARQ development codebase, run the example from sparqlProcessor.txt as best I can and it works for me (I get legal JSON output). I used a query of DESCRIBE < http://www.w3.org/2004/02/skos/core#prefLabel > and DESCRIBE < http://api.talis.com/stores/mesh-norwegian/items/D016315 > both of which have several skos:alkLabels. Rurik - please provide an complete minimal example (ideally not using Talis's remote service as this is unrelated to the RDFJSON out on a local graph). Extracts from other system that aren't standalone make it hard to know if some other factor is involved for example other handling of the HttpServletResponse. I propose we apply these patches.
          Hide
          Rob Vesse added a comment -

          Your code usage pattern is essentially identical to the one in the unit tests and I am unable to reproduce this still, I still can't find anything wrong with the code for writing RDF/JSON so I'm not sure how to proceed with this.

          Can someone from the Jena team please test the patches and see if anyone else can reproduce this?

          Show
          Rob Vesse added a comment - Your code usage pattern is essentially identical to the one in the unit tests and I am unable to reproduce this still, I still can't find anything wrong with the code for writing RDF/JSON so I'm not sure how to proceed with this. Can someone from the Jena team please test the patches and see if anyone else can reproduce this?
          Hide
          Rurik Thomas Greenall added a comment -

          The code I'm using (has been edited to remove the Spring-based config for the SparqlURI, but is otherwise intact).

          Show
          Rurik Thomas Greenall added a comment - The code I'm using (has been edited to remove the Spring-based config for the SparqlURI, but is otherwise intact).
          Hide
          Rob Vesse added a comment -

          Or maybe not, looked into my theory and that is not the source of the dud JSON, still can't reproduce so still need to see code sample please?

          Show
          Rob Vesse added a comment - Or maybe not, looked into my theory and that is not the source of the dud JSON, still can't reproduce so still need to see code sample please?
          Hide
          Rob Vesse added a comment -

          Can you post a full code example regardless?

          I just noticed something about the code that might explain the behavior you see but as I haven't been able to reproduce it I really need to see exactly how you invoke the code to confirm my suspicions

          Show
          Rob Vesse added a comment - Can you post a full code example regardless? I just noticed something about the code that might explain the behavior you see but as I haven't been able to reproduce it I really need to see exactly how you invoke the code to confirm my suspicions
          Hide
          Rurik Thomas Greenall added a comment - - edited

          I'm using trunk from github.
          I'm not doing anything except passing model.getGraph() to RDFJSONWriter.write(out,graph); I'm sure, though that there is a problem with the local setup here if you're not experiencing the same issues. Is there a chance of you guys providing a snapshot with these patches so that I can test this?

          Show
          Rurik Thomas Greenall added a comment - - edited I'm using trunk from github. I'm not doing anything except passing model.getGraph() to RDFJSONWriter.write(out,graph); I'm sure, though that there is a problem with the local setup here if you're not experiencing the same issues. Is there a chance of you guys providing a snapshot with these patches so that I can test this?
          Hide
          Rob Vesse added a comment -

          You applied the patches against ARQ trunk from the Apache repository I assume rather than the old SourceForge repositories which are no longer updated?

          I've looked over the writer code again and can't see any obvious reason why it should produce invalid JSON like you saw, can you provide a minimal code sample that shows this behaviour?

          All the unit tests present for RDF/JSON still pass with these patches so either there is something different in your environment or you're hitting a corner case that the unit tests don't cover.

          I even created an additional unit test that generates the graph you are attempting to serialize based on your attached JSON fragment and that passed OK so not sure what is going on with this until I can see a fuller example of how the graph is generated and the writer is invoked.

          Show
          Rob Vesse added a comment - You applied the patches against ARQ trunk from the Apache repository I assume rather than the old SourceForge repositories which are no longer updated? I've looked over the writer code again and can't see any obvious reason why it should produce invalid JSON like you saw, can you provide a minimal code sample that shows this behaviour? All the unit tests present for RDF/JSON still pass with these patches so either there is something different in your environment or you're hitting a corner case that the unit tests don't cover. I even created an additional unit test that generates the graph you are attempting to serialize based on your attached JSON fragment and that passed OK so not sure what is going on with this until I can see a fuller example of how the graph is generated and the writer is invoked.
          Hide
          Rurik Thomas Greenall added a comment -

          I applied the patches locally, but the writer now provides consistently invalid JSON. See the attached example of the JSON produced.

          Show
          Rurik Thomas Greenall added a comment - I applied the patches locally, but the writer now provides consistently invalid JSON. See the attached example of the JSON produced.
          Hide
          Paolo Castagna added a comment -

          Thank you Andy for creating the issue.
          Thank you Rob for the patch.

          > Whoever wrote the patch that allowed for writing RDF/JSON completey forgot to insert array separators into the object list array, this patch fixes this.

          That's me, late at night probably.

          I'll look at this in the next couple of days (unless it's urgent and someone else want to check and apply Rob's patch before)

          Show
          Paolo Castagna added a comment - Thank you Andy for creating the issue. Thank you Rob for the patch. > Whoever wrote the patch that allowed for writing RDF/JSON completey forgot to insert array separators into the object list array, this patch fixes this. That's me, late at night probably. I'll look at this in the next couple of days (unless it's urgent and someone else want to check and apply Rob's patch before)
          Hide
          Rob Vesse added a comment -

          Whoever wrote the patch that allowed for writing RDF/JSON completey forgot to insert array separators into the object list array, this patch fixes this.

          This should allow this issue to be closed as fixed once this and the associated parser patch are integrated

          Show
          Rob Vesse added a comment - Whoever wrote the patch that allowed for writing RDF/JSON completey forgot to insert array separators into the object list array, this patch fixes this. This should allow this issue to be closed as fixed once this and the associated parser patch are integrated
          Hide
          Rob Vesse added a comment -

          Patch which fixes the RDF/JSON parser to properly throw an error in the case of a missing comma in the object list array

          Show
          Rob Vesse added a comment - Patch which fixes the RDF/JSON parser to properly throw an error in the case of a missing comma in the object list array
          Hide
          Andy Seaborne added a comment -

          Test case, as an application and as a JUnit test.

          Show
          Andy Seaborne added a comment - Test case, as an application and as a JUnit test.
          Hide
          Andy Seaborne added a comment -

          Attached file illustrates the problem

          This print-parse can be used for JUnit testing as well.

          Show
          Andy Seaborne added a comment - Attached file illustrates the problem This print-parse can be used for JUnit testing as well.

            People

            • Assignee:
              Paolo Castagna
              Reporter:
              Andy Seaborne
            • Votes:
              2 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development