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 <mor...@dhis2.org> 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 <halv...@dhis2.org> > 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 <mor...@dhis2.org> >> 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 <nico...@dhis2.org> 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 <mor...@dhis2.org> >>>> 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 <mor...@dhis2.org> >>>>> wrote: >>>>> >>>>>> >>>>>> On Thu, Jun 9, 2016 at 12:45 PM, Morten Olav Hansen <mor...@dhis2.org >>>>>> > 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@lists.launchpad.net >>>>> 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/> >> >> >
-- Mailing list: https://launchpad.net/~dhis2-devs-core Post to : dhis2-devs-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~dhis2-devs-core More help : https://help.launchpad.net/ListHelp
