← Back to team overview

dhis2-devs-core team mailing list archive

Re: ApiVersioning redux

 

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

Follow ups

References