Spring Roo
  1. Spring Roo
  2. ROO-3376

Generated controllers violate W3C HTTP PUT specification

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Complete
    • Affects Version/s: 1.2.3.RELEASE
    • Fix Version/s: 1.2.4.RELEASE
    • Component/s: WEB MVC
    • Labels:
      None

      Description

      The JSON controllers generated by Roo violate the specification of the HTTP PUT method defined by W3C (see http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html)

      The URL called for PUT has to be a single resource e.g 'myServer/persons/1' where '1' is the unique identifier of a person resource.

      If the resource exists, it gets replaced, else it will be created with the given identifier.

      The SpringMVC code generated by Roo maps HTTP PUT requests 'myServer/persons' to the updateFromJson method. The unique identifier is parsed from the body and is not in the url. This is wrong and most RESTful clients will call PUT with the identifier in the URL.

      Things are even worse because Roo also creates a showJson method mapped to 'myServer/persons/1' which is NOT limited to a GET request.

      This results in getting the showJson method called instead of the updateFromJson method if a proper PUT request with identifier is sent. This makes the use of most of the existing client-side REST frameworks like AngularJS very painful because every controller has to be corrected "by hand".

      I attached a Roo script which creates a (wrong) PersonController and I also attached a corrected PersonController that IMHO fits the W3C specification.

      BTW: There are other REST dependent issues with like not returning the link of the created resource in createFromJson (HATEOAS) but these will no be part of this issue (Spring Data REST creates quite good REST controllers btw.)

      1. PersonController_Roo_Controller_Json.aj
        4 kB
        Roger Villars
      2. pizzashop_test.sh
        3 kB
        Alan Stewart
      3. restapp.roo
        0.4 kB
        Roger Villars
      1. diff_1_of_3.jpg
        192 kB
      2. diffs_2_and_3_of_3.jpg
        224 kB

        Activity

        Hide
        Dave McLure added a comment -

        OK, now I see the problem - the problem is that the curl PUT statement in the existing roo_deploy.sh does not include the id of the json object being put. This is one of the things which I added to roo in order to comply with the REST spec. The current curl statement is just trying to PUT to the entire list as follows:

        "Existing PUT request"
        curl -i -s -X PUT -H "Content-Type: application/json" -H "Accept: application/json" -o /tmp/rootest/curl.txt -d "{id:6,name:\"Mozzarella\",version:1}" http://localhost:8888/pizzashop/toppings
        

        Instead, if we simply add "/6" to the end, as we correctly do later in the GET of that particular element, we get the desired results:

        "Fixed PUT request"
        curl -i -s -X PUT -H "Content-Type: application/json" -H "Accept: application/json" -o /tmp/rootest/curl.txt -d "{id:6,name:\"Mozzarella\",version:1}" http://localhost:8888/pizzashop/toppings/6
        

        It does seem a little redundant that the id is also included in the payload, but I think this is how we are supposed to do this. See http://stackoverflow.com/questions/4477454/how-is-a-http-put-request-typically-issued and http://blogs.plexibus.com/2009/01/15/rest-esting-with-curl/ for a couple of discussions around this.

        Show
        Dave McLure added a comment - OK, now I see the problem - the problem is that the curl PUT statement in the existing roo_deploy.sh does not include the id of the json object being put. This is one of the things which I added to roo in order to comply with the REST spec. The current curl statement is just trying to PUT to the entire list as follows: "Existing PUT request" curl -i -s -X PUT -H "Content-Type: application/json" -H "Accept: application/json" -o /tmp/rootest/curl.txt -d "{id:6,name:\" Mozzarella\ ",version:1}" http: //localhost:8888/pizzashop/toppings Instead, if we simply add "/6" to the end, as we correctly do later in the GET of that particular element, we get the desired results: "Fixed PUT request" curl -i -s -X PUT -H "Content-Type: application/json" -H "Accept: application/json" -o /tmp/rootest/curl.txt -d "{id:6,name:\" Mozzarella\ ",version:1}" http: //localhost:8888/pizzashop/toppings/6 It does seem a little redundant that the id is also included in the payload, but I think this is how we are supposed to do this. See http://stackoverflow.com/questions/4477454/how-is-a-http-put-request-typically-issued and http://blogs.plexibus.com/2009/01/15/rest-esting-with-curl/ for a couple of discussions around this.
        Hide
        Alan Stewart added a comment -

        I've updated roo-deploy.sh with the above curl and the script completes without errors now.

        There is one last curl statement that is commented out, and I think it never worked. If you're willing to have a go at fixing it, please feel free. It creates a pizzaorder. PizzaOrder has a composite primary key.

        curl -i -s -X POST -H "Content-Type: application/json" -H "Accept: application/json" -o /tmp/rootest/curl.txt -d "{name:\"Stefan\",total:7.5,address:\"Sydney, AU\",deliveryDate:1314595427866,id:{shopCountry:\"AU\",shopCity:\"Sydney\",shopName:\"Pizza Pan 1\"},pizzas:[{id:8,version:1}]}" http://localhost:8888/pizzashop/pizzaorders	
        

        - line 303 of roo-deploy.sh now

        Show
        Alan Stewart added a comment - I've updated roo-deploy.sh with the above curl and the script completes without errors now. There is one last curl statement that is commented out, and I think it never worked. If you're willing to have a go at fixing it, please feel free. It creates a pizzaorder. PizzaOrder has a composite primary key. curl -i -s -X POST -H "Content-Type: application/json" -H "Accept: application/json" -o /tmp/rootest/curl.txt -d "{name:\" Stefan\ ",total:7.5,address:\" Sydney, AU\ ",deliveryDate:1314595427866,id:{shopCountry:\" AU\ ",shopCity:\" Sydney\ ",shopName:\" Pizza Pan 1\ "},pizzas:[{id:8,version:1}]}" http: //localhost:8888/pizzashop/pizzaorders - line 303 of roo-deploy.sh now
        Hide
        Dave McLure added a comment - - edited

        I think one reason that the more complex pizzashop post operations aren't working is because if you take a look at the pizzashop.roo which is provided in the roo resources directory, there is no "toppings" element included in a Pizza element. A Pizza entity currently only seems to consist of a name and a price:

        "Current Pizza Entity (in pizzashop.roo)"
        entity jpa --class ~.domain.Pizza --testAutomatically
        field string --fieldName name --notNull --sizeMin 2
        field number --fieldName price --type java.lang.Float
        

        So when you try to include toppings on a pizza as a part of a PizzaOrder entity, things get a little confused.

        When I add a reference to a set of toppings to a pizza like this:

        "Fixed Pizza Entity (in pizzashop.roo)"
        entity jpa --class ~.domain.Pizza --testAutomatically
        field string --fieldName name --notNull --sizeMin 2
        field number --fieldName price --type java.lang.Float
        field set --fieldName toppings --type ~.domain.Toppings
        

        ..I can get through the first of the two complex post tests (pizzas) with a 200 result:

        ""
        curl -i -s -X POST -H "Content-Type: application/json" -H "Accept: application/json"  -o /tmp/rootest/curl.txt -d "{name:\"Napolitana\",price:7.5,base:{id:1},toppings:[{name: \"Anchovy fillets\"},{name: \"Mozzarella\"}]}" http://localhost:8888/pizzashop/pizzas
        

        ...you can see how it successfully adds the new "Napolitana" pizza to pizzas, as well as the new "Anchovy fillets" topping to toppings. This seemed OK at first, but I guess I would really want to see a 201 [CREATED] return here instead.

        I can also get through the second complex post test (pizzaorders) with a 200 result, but an error still occurs shortly after this command. If you leave the second test commented out, the rest of the test script seems to complete normally, so that would sort of implicate the second of the two complex post statements.

        In fact, the JPA error only occurs after both tests have completed, and just as the Tomcat server is being terminated, so the actual error may have more to do with the way the tomcat server is being terminated than anything else.

        One other thing to consider is that these (ROO-3376) roo changes shouldn't effect existing post behavior. The only thing approaching post behavior which was removed was a PUT of a list of items - which really kind of goes against the REST spec according to the original defect report - as a list should probably either be posted, PUT as an object with its own id, or individually PUT one element at a time. That is not to say that we don't still have an issue here, it's just that it doesn't really seem like part of this Jira.

        Show
        Dave McLure added a comment - - edited I think one reason that the more complex pizzashop post operations aren't working is because if you take a look at the pizzashop.roo which is provided in the roo resources directory, there is no "toppings" element included in a Pizza element. A Pizza entity currently only seems to consist of a name and a price: "Current Pizza Entity (in pizzashop.roo)" entity jpa --class ~.domain.Pizza --testAutomatically field string --fieldName name --notNull --sizeMin 2 field number --fieldName price --type java.lang. Float So when you try to include toppings on a pizza as a part of a PizzaOrder entity, things get a little confused. When I add a reference to a set of toppings to a pizza like this: "Fixed Pizza Entity (in pizzashop.roo)" entity jpa --class ~.domain.Pizza --testAutomatically field string --fieldName name --notNull --sizeMin 2 field number --fieldName price --type java.lang. Float field set --fieldName toppings --type ~.domain.Toppings ..I can get through the first of the two complex post tests (pizzas) with a 200 result: "" curl -i -s -X POST -H "Content-Type: application/json" -H "Accept: application/json" -o /tmp/rootest/curl.txt -d "{name:\" Napolitana\ ",price:7.5,base:{id:1},toppings:[{name: \" Anchovy fillets\ "},{name: \" Mozzarella\ "}]}" http: //localhost:8888/pizzashop/pizzas ...you can see how it successfully adds the new "Napolitana" pizza to pizzas, as well as the new "Anchovy fillets" topping to toppings. This seemed OK at first, but I guess I would really want to see a 201 [CREATED] return here instead. I can also get through the second complex post test (pizzaorders) with a 200 result, but an error still occurs shortly after this command. If you leave the second test commented out, the rest of the test script seems to complete normally, so that would sort of implicate the second of the two complex post statements. In fact, the JPA error only occurs after both tests have completed, and just as the Tomcat server is being terminated, so the actual error may have more to do with the way the tomcat server is being terminated than anything else. One other thing to consider is that these ( ROO-3376 ) roo changes shouldn't effect existing post behavior. The only thing approaching post behavior which was removed was a PUT of a list of items - which really kind of goes against the REST spec according to the original defect report - as a list should probably either be posted, PUT as an object with its own id, or individually PUT one element at a time. That is not to say that we don't still have an issue here, it's just that it doesn't really seem like part of this Jira.
        Hide
        Dave McLure added a comment - - edited

        The net result is, this Jira is stuck with a remaining complex curl statement that seems to somehow cause a JPA error at some point after it [successfully?] completes with a 200 result:

        "Problematic complex POST #2"
        curl -i -s -X POST -H "Content-Type: application/json" -H "Accept: application/json" -o /tmp/rootest/curl.txt -d "{name:\"Stefan\",total:7.5,address:\"Sydney, AU\",deliveryDate:1314595427866,id:{shopCountry:\"AU\",shopCity:\"Sydney\",shopName:\"Pizza Pan 1\"},pizzas:[{id:8,version:1}]}" http://localhost:8888/pizzashop/pizzaorders     
        

        Here is the pizzaorders data from the curl statement formatted in hopes of eyeballing whether or not all of these entities are accurately represented here in json:

        "pizzaorders data from complex post #2"
            name:\"Stefan\",
            total:7.5,
            address:\"Sydney, AU\",
            deliveryDate:1314595427866,
            id:{
                shopCountry:\"AU\",
                shopCity:\"Sydney\",
                shopName:\"Pizza Pan 1\"
            },
            pizzas:[{id:8,version:1}]
        

        I'm also trying to better understand the PizzaOrder Entity defined in pizzashop.roo:

        "PizzaOrder Entity defined in pizzashop.roo"
        entity jpa --class ~.domain.PizzaOrder --testAutomatically --activeRecord false --identifierType ~.domain.PizzaOrderPk
        field string --fieldName name --notNull --sizeMin 2
        field string --fieldName address --sizeMax 30
        field number --fieldName total --type java.math.BigDecimal
        field date --fieldName deliveryDate --type java.util.Date
        field set --fieldName pizzas --type ~.domain.Pizza
        
        field string --fieldName shopCountry --class ~.domain.PizzaOrderPk
        field string --fieldName shopCity
        field string --fieldName shopName
        

        This was when I noticed the odd looking PizzaOrderPk class definition tacked onto the. shopCountry fieldname somehow. Is this legal roo syntax? If so, then what is it? Some sort of inner class? Or shouldn't the PizzaOrderPk class actually be defined as an Entity like other classes? This just seemed a little weird...

        Show
        Dave McLure added a comment - - edited The net result is, this Jira is stuck with a remaining complex curl statement that seems to somehow cause a JPA error at some point after it [successfully?] completes with a 200 result: "Problematic complex POST #2" curl -i -s -X POST -H "Content-Type: application/json" -H "Accept: application/json" -o /tmp/rootest/curl.txt -d "{name:\" Stefan\ ",total:7.5,address:\" Sydney, AU\ ",deliveryDate:1314595427866,id:{shopCountry:\" AU\ ",shopCity:\" Sydney\ ",shopName:\" Pizza Pan 1\ "},pizzas:[{id:8,version:1}]}" http: //localhost:8888/pizzashop/pizzaorders Here is the pizzaorders data from the curl statement formatted in hopes of eyeballing whether or not all of these entities are accurately represented here in json: "pizzaorders data from complex post #2" name:\ "Stefan\" , total:7.5, address:\ "Sydney, AU\" , deliveryDate:1314595427866, id:{ shopCountry:\ "AU\" , shopCity:\ "Sydney\" , shopName:\ "Pizza Pan 1\" }, pizzas:[{id:8,version:1}] I'm also trying to better understand the PizzaOrder Entity defined in pizzashop.roo: "PizzaOrder Entity defined in pizzashop.roo" entity jpa --class ~.domain.PizzaOrder --testAutomatically --activeRecord false --identifierType ~.domain.PizzaOrderPk field string --fieldName name --notNull --sizeMin 2 field string --fieldName address --sizeMax 30 field number --fieldName total --type java.math.BigDecimal field date --fieldName deliveryDate --type java.util.Date field set --fieldName pizzas --type ~.domain.Pizza field string --fieldName shopCountry --class ~.domain.PizzaOrderPk field string --fieldName shopCity field string --fieldName shopName This was when I noticed the odd looking PizzaOrderPk class definition tacked onto the. shopCountry fieldname somehow. Is this legal roo syntax? If so, then what is it? Some sort of inner class? Or shouldn't the PizzaOrderPk class actually be defined as an Entity like other classes? This just seemed a little weird...
        Hide
        Alan Stewart added a comment -

        Dave - thanks for your effort on this. In fact this Jira is not affected by the failing curl statement as I described. This and previous curl statements did not work due to reasons I did not fully investigate when the team member who created the the json add-on, Stefan Schmidt, left VMware.
        I'm resolving this now as complete ready for 1.2.4 release by end June '13
        Any extra info in actually creating a pizzaorder using curl would be a bonus!
        Alan

        Show
        Alan Stewart added a comment - Dave - thanks for your effort on this. In fact this Jira is not affected by the failing curl statement as I described. This and previous curl statements did not work due to reasons I did not fully investigate when the team member who created the the json add-on, Stefan Schmidt, left VMware. I'm resolving this now as complete ready for 1.2.4 release by end June '13 Any extra info in actually creating a pizzaorder using curl would be a bonus! Alan

          People

          • Assignee:
            Alan Stewart
            Reporter:
            Roger Villars
          • Votes:
            7 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: