Re: [Bigdata-developers] 711 and 704 fix (Protocol issues)

[Bryan T. <br...@sy...>]

Jeremy,

I was able to specify to the eclipse patch tool that it should ignore the 1st path component for the patch. That appears to have produced a reasonable outcome.  (The diff has leading a/ and b/ path prefixes.)

- I tend to be a fanatic about using 'final' whenever a variable is immutable.  Likewise, I like to leave vertical whitespace between methods and common their intention.

- I tend to link the code back to the ticket when fixing a bug or creating a new test/test suite.

- Please update the 2.7 migration ticket to link back to the code change that will be required in ConnegUtil.

Pedantics aside, this looks fine.  I am not sure whether or not we already have some of those methods.

Looks like several excellent finds in there!  Please go ahead and commit.

Thanks,
Bryan


From: Jeremy Carroll <jj...@sy...<mailto:jj...@sy...>>
Date: Friday, August 9, 2013 1:27 PM
To: "Big...@li...<mailto:Big...@li...>" <Big...@li...<mailto:Big...@li...>>
Subject: [Bigdata-developers] 711 and 704 fix (Protocol issues)

I have attached a patch to trac 711 which:
- fixes 711 (POST of application/sparql-query)
- fixes 704 (ASK and json)
- provides tests for both
- provides test framework for 'protocol' tests
- makes it easier to run on-the-wire tests

This message is to ask for review, and any further instructions.

I have two question for the reviewer(s)
 - about a new utility I/O method
 - about a not quite relate change to MiniMime

OVERVIEW

I provide a summary of my changes by reference to the diff on github
https://github.com/jeremycarroll/bigdata/compare/fd7d9e0...trac-711-post
- Actual fixes to:
   704
    ConnegUtil.java
  711
    BigdataRDFContext.java
    QueryServlet.java (with question I/O utility)
    RESTServlet.java
  other
    MiniMime.java (with question not this bug)

- Test enhancement
   TestConneg.java
   AbstractProtocolTest.java
         ExampleProtocolTest.java
         TestAskJsonTrac704.java
         TestPostNotURLEncoded.java
         TestRelease123Protocol.java

- Test launching enhancements
   ProxySuiteHelper.java
   TestProtocolAll.java


- Actual fixes to:
   704
    ConnegUtil.java
https://github.com/jeremycarroll/bigdata/compare/fd7d9e0...d53f287#diff-2
    The new code is lines 66 - 99 with line 92 being the heart of it
    I include line 70 that disables the fix and issues a warning once sesame upgrade is in place


  711
    BigdataRDFContext.java

Missing mime type for update responses

   QueryServlet.java (with question I/O utility)
    RESTServlet.java

Use mime type as well as params for control flow POST requests; and use body for query not parameters, where appropriate.
This does not check for multiply defined queries (e.g. both in body and on parameter) or other bad cases

In QueryServlet there is a method readFully which takes a Reader and returns a String of its content.
https://github.com/jeremycarroll/bigdata/compare/fd7d9e0...d53f287#diff-3 line 67
Do we already have such a method? Should this method be moved to a utility class?


  other
    MiniMime.java (with question not this bug)

I noticed that this class makes assumptions about the absence of whitespace that is permitted by the relevant RFC.
I fixed that in one detail (the content-type) but not in the other detail (the q processing).
This was not part of defect 711 or 704; and there is no motivating test.

- Test enhancement
   TestConneg.java

   Additional test related to 704

   AbstractProtocolTest.java

    Provides fairly low level control of the wire protocol for protocol related tests rather than query related tests.
    The intent is to subclass as follows, and for this class to be largely a black box ….

         ExampleProtocolTest.java
               A very simple example of how to use the superclass.

         TestRelease123Protocol.java
               A set of tests that capture some aspects of the  behavior for queries and sparql update of the current version.
               Note: these caught the missing mime type in BigdataRDFContent

         TestAskJsonTrac704.java
               Simple test for 704


         TestPostNotURLEncoded.java
                 These are the tests for 711

- Test launching enhancements
   ProxySuiteHelper.java
       This provides two methods encapsulating the 'how to launch tests' advice from this week's meeting, in a way that tries to avoid temporary copies and kludges during development.
       Each of the above test classes can be launched stand-alone without change.

   TestProtocolAll.java
         A test suite of all the above tests, that using the glue from the ProxySuiteHelper can also run successfully in TestAll, as well as stand-alone




Jeremy J Carroll
Principal Architect
Syapse, Inc.