dhis2-devs-core team mailing list archive

[Morten Olav Hansen <morten@xxxxxxxxx>]

Halvdan

I haven't been able to reproduce all of your issues, but I have added some
more tests.. when you merge in this branch, please let me know.. and I will
test against programStageNotifications endpoint directly, all of
/api/23/programStageNotifications should of course return 404 in this case

-- 
Morten Olav Hansen
Senior Engineer, DHIS 2
University of Oslo
http://www.dhis2.org

On Fri, Jun 10, 2016 at 9:24 AM, Morten Olav Hansen <morten@xxxxxxxxx>
wrote:

> Regarding 404 vs 405, that's up to Spring MVC, if there is at least one
> method mapped at the endpoint, it will usually return 405, if not 404.. I'm
> looking into this, and adding a few new use-cases to the testing suite
>
> --
> Morten Olav Hansen
> Senior Engineer, DHIS 2
> University of Oslo
> http://www.dhis2.org
>
> On Thu, Jun 9, 2016 at 5:12 PM, Halvdan Hoem Grelland <halvdan@xxxxxxxxx>
> wrote:
>
>> To be clear:
>>
>> I have a (new) generic controller, as seen below:
>>
>> @Controller
>> @RequestMapping( value = ProgramStageNotificationSchemaDescriptor.API_ENDPOINT )
>> @ApiVersion( include = { ApiVersion.Version.V24 } )
>> public class ProgramStageNotificationController
>>     extends AbstractCrudController<ProgramStageNotification>
>> {
>> }
>>
>> Deducting from the discussions we've had this should clearly only work on
>> 24, right?
>>
>> To break it down, this is what's returned for different methods:
>>
>>
>>    - GET api/programStageNotifications --> 404 not found ✓
>>    - GET api/23/programStageNotifications --> 405 request method not
>>    supported ✗ ?
>>    - GET api/24/programStageNotifications --> 200 ✓
>>
>> Except for the 405 vs 404 inconsistency this is fine, I guess? Is this a
>> servlet container-specific thing? Running on Jetty.
>>
>> Further:
>>
>>
>>    - POST api/programStageNotifications --> 404 not found ✓
>>    - POST api/23/programStageNotifications --> 200 ✗
>>    - POST api/24/programStageNotifications --> 201 ✓
>>
>>
>> This is with the latest patches on trunk.
>>
>> On Thu, Jun 9, 2016 at 11:35 AM, Morten Olav Hansen <morten@xxxxxxxxx>
>> wrote:
>>
>>> Hi Nicolay
>>>
>>> The main use-case for exclude was to remove versions that was added to
>>> the type level, so if you class was annotated with `@ApiVersion(V23, V24)`
>>> you could add `@ApiVersion(exclude = V23)` on specific methods.
>>>
>>> I think it could be added for `ALL` also, it's just not there yet. Let
>>> me have a look tomorrow.
>>>
>>> --
>>> Morten Olav Hansen
>>> Senior Engineer, DHIS 2
>>> University of Oslo
>>> http://www.dhis2.org
>>>
>>> On Thu, Jun 9, 2016 at 4:26 PM, Nicolay Ramm <nicolay@xxxxxxxxx> wrote:
>>>
>>>> Hi Morten,
>>>>
>>>> this problem surfaced when I added an endpoint for updating custom
>>>> forms by POSTing json to /api/*/dataSets/{uid}/form
>>>>
>>>> This after discussing with Halvdan, I annotated the endpoint with the
>>>> following version:
>>>>
>>>> @ApiVersion( value = ApiVersion.Version.ALL, exclude = ApiVersion.Version.V23 )
>>>>
>>>> I would expect this to mean that the endpoint should work for default
>>>> and 24, but not for 23. In practice however, the endpoint works for 23 as
>>>> well.
>>>>
>>>> To be painfully explicit, I would expect this to work:
>>>>
>>>> curl -u admin:district :8080/api/dataSets/BfMAe6Itzgt/form -X POST -H
>>>> 'content-type: application/json' -d '{"display":"NORMAL"}' -v
>>>> As well as this:
>>>>
>>>> curl -u admin:district :8080/api/24/dataSets/BfMAe6Itzgt/form -X POST
>>>> -H 'content-type: application/json' -d '{"display":"NORMAL"}' -v
>>>> But *not* this:
>>>>
>>>> curl -u admin:district :8080/api/23/dataSets/BfMAe6Itzgt/form -X POST
>>>> -H 'content-type: application/json' -d '{"display":"NORMAL"}' -v
>>>>
>>>> This is not really a problem in this case of course. But I guess that
>>>> adding new endpoints that shouldn't work on older versions will be a pretty
>>>> common use case for API versioning, so this is probably a useful test case.
>>>>
>>>>
>>>> Nicolay Ramm
>>>> Front end developer, DHIS 2
>>>> University of Oslo
>>>> https://www.dhis2.org
>>>>
>>>> On Thu, Jun 9, 2016 at 10:07 AM, Morten Olav Hansen <morten@xxxxxxxxx>
>>>> wrote:
>>>>
>>>>> Hi Halvdan
>>>>>
>>>>> This should be fixed now, the issue was that even though
>>>>> @RequestMapping will default to GET (if not method is set) the annotation
>>>>> doesn't default to GET. I'm now mapping method = {} to method = GET
>>>>> internally, which means that your issues should be gone.
>>>>>
>>>>> I think there will be similar issues if you want and endpoint for
>>>>> application/json to have a different version than application/xml, but I
>>>>> think that is taking this versioning a bit far.
>>>>>
>>>>> --
>>>>> Morten Olav Hansen
>>>>> Senior Engineer, DHIS 2
>>>>> University of Oslo
>>>>> http://www.dhis2.org
>>>>>
>>>>> On Thu, Jun 9, 2016 at 12:51 PM, Morten Olav Hansen <morten@xxxxxxxxx>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>> On Thu, Jun 9, 2016 at 12:45 PM, Morten Olav Hansen <morten@xxxxxxxxx
>>>>>> > wrote:
>>>>>>
>>>>>>> Not 100% sure what you mean here, we already have POST/PUT with
>>>>>>> different methods on / vs /24 etc (for same mapping)... but I will try it
>>>>>>> out, maybe there are some bugs there
>>>>>>>
>>>>>>
>>>>>> Hm, I made some tests around this now.. and I see there are some
>>>>>> issues, will try to fix today or tomorrow (working on it now)
>>>>>>
>>>>>> --
>>>>>> Morten Olav Hansen
>>>>>> Senior Engineer, DHIS 2
>>>>>> University of Oslo
>>>>>> http://www.dhis2.org
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Mailing list: https://launchpad.net/~dhis2-devs-core
>>>>> Post to     : dhis2-devs-core@xxxxxxxxxxxxxxxxxxx
>>>>> Unsubscribe : https://launchpad.net/~dhis2-devs-core
>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>
>>>>>
>>>>
>>>
>>
>>
>> --
>> Halvdan Hoem Grelland
>> Software developer, DHIS 2
>> University of Oslo
>> http://www.dhis2.org <https://www.dhis2.org/>
>>
>>
>