← Back to team overview

dhis2-devs-core team mailing list archive

Re: ApiVersioning redux

 

OK I will.

On Mon, Jun 13, 2016 at 4:27 AM, Morten Olav Hansen <morten@xxxxxxxxx>
wrote:

> 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/>
>>>
>>>
>>
>


-- 
Halvdan Hoem Grelland
Software developer, DHIS 2
University of Oslo
http://www.dhis2.org <https://www.dhis2.org/>

References