On Wed, 22 Sept 2021 at 19:20, Ali Alhaidary <ali.alhaid...@the5stars.org> wrote:
> > On 9/22/21 3:14 PM, Maxim Solodovnik wrote: > > I only can do manual testing here :( > > What is manual testing? > I'm installing everything setting up integration and do clicking :))) > IMO these changes (if we will be able to do them) worth to be done > > what is IMO ? > In My Opinion :) > Why I raise some old design issues: we can do changes now and let the API > unchanged for another several years :))) > > What is several years ;-) > Well I believe REST API was changed 2-3 times, so we are trying to keep it stable v1/v2 etc. approach can also be applied the problem here: I don't have enough time to maintain more than one version :(( > On Wed, 22 Sept 2021 at 19:09, Ali Alhaidary <ali.alhaid...@the5stars.org> > wrote: > >> The issue here is that, It is a lot of work, and, a lot of testing that >> follows. We are not a direct API users, however, moodle plugin is. Along >> the road, things could break in such change. So, if you see this change is >> the the way forward, I am in with as usual a dedicated production server >> for selected teaches/students as long as the old work (mainly recordings) >> is not lost, and, only one environment is used (as is now), i.e. moodle >> plugin can handle all the communication. >> >> The issue is being discussed by only three people, how many others are >> using these APIs ? How many apps are up and running on them now ? looking >> at the moodle plugin downloads, >> https://moodle.org/plugins/mod_openmeetings/stats there is a peak during >> the past year, and I am sure the case is the same with other LMS and custom >> built apps, keeping in mind that OM can work exceptional good by itself. >> >> Ali >> >> >> On 9/22/21 2:16 PM, Maxim Solodovnik wrote: >> >> These changes are only being discussed >> Nothing is broken, yet :)))) >> we can @Deprecate these old methods and/or move it to some prefixed URL >> so API users will need to change base URL from >> https://localhost:5443/openmeetings to >> https://localhost:5443/openmeetings/v1 >> >> On Wed, 22 Sept 2021 at 13:14, seba.wag...@gmail.com < >> seba.wag...@gmail.com> wrote: >> >>> @Ali Alhaidary <ali.alhaid...@the5stars.org> >>> The other alternative to fix the issue AND make it backwards compatible >>> would be to have a /v2 version of the API >>> >>> So all endpoints would be duplicated to have version /v2 of the API >>> (with maybe some other fixes) >>> and the current API stays the same. But would not receive any >>> improvements anymore/deprecated. >>> >>> But that would be quite a bit of work. But yeah, that is what people do >>> when they want to avoid breaking changes. Need to do versioning. >>> >>> Thanks >>> Seb >>> >>> >>> Sebastian Wagner >>> Director Arrakeen Solutions, OM-Hosting.com >>> http://arrakeen-solutions.co.nz/ >>> https://om-hosting.com - Cloud & Server Hosting for HTML5 >>> Video-Conferencing OpenMeetings >>> >>> <https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url> >>> <https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url> >>> >>> >>> On Wed, 22 Sept 2021 at 18:10, Ali Alhaidary < >>> ali.alhaid...@the5stars.org> wrote: >>> >>>> We are using OM in production with moodle front end, we can not >>>> tolerate downtime neither with OM or its plugin (that needs fixing, but >>>> living with), and to tell you the truth, I do not see it as 'broken' from >>>> that angle. >>>> >>>> So my answer is B. >>>> >>>> Ali >>>> On 9/22/21 2:10 AM, seba.wag...@gmail.com wrote: >>>> >>>> It is broken. The problem is the fix will be a breaking change that >>>> will require 3rd party integration code to be fixed. Not a big fix, but a >>>> fix. Eg the Moodle Plugin requires some minor changes. >>>> >>>> The workaround is to write some additional wrapper code to make it >>>> backwards compatible. Which is also a bit confusing. >>>> >>>> I also don't understand quite if you answer is pro or contra changing >>>> the response. >>>> >>>> So is your statement: >>>> A) Yes, lets fix it to align our JSON response with what the >>>> schema/method signature says. We don't like wrapper objects. And I am happy >>>> that people have to change their integration code to use newer versions of >>>> OpenMeetings. >>>> B) No, lets leave it like this for now and we do whatever other >>>> additional code we need to write to workaround so that our documentation >>>> and schema matches what the actual API responses look like >>>> >>>> If you could please clarify if you are A, B. Or if you don't mind >>>> either way/no strong opinion :) >>>> >>>> Thanks >>>> Seb >>>> >>>> >>>> Sebastian Wagner >>>> Director Arrakeen Solutions, OM-Hosting.com >>>> http://arrakeen-solutions.co.nz/ >>>> https://om-hosting.com - Cloud & Server Hosting for HTML5 >>>> Video-Conferencing OpenMeetings >>>> >>>> <https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url> >>>> <https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url> >>>> >>>> >>>> On Wed, 22 Sept 2021 at 10:59, Ali Alhaidary < >>>> ali.alhaid...@the5stars.org> wrote: >>>> >>>>> Hi, >>>>> >>>>> We have an old saying 'If it is not broken, do not fix it' ;-) >>>>> >>>>> Ali >>>>> On 9/22/21 12:46 AM, seba.wag...@gmail.com wrote: >>>>> >>>>> Hi, >>>>> >>>>> as discussed in the comments section in >>>>> https://github.com/apache/openmeetings/commit/4daf7c1f53738cd786dc976114cc5278b4f05f4f#comments >>>>> >>>>> >>>>> we would like to propose a breaking change for the OpenMeetings >>>>> Json/Rest API in v7.0.0 >>>>> >>>>> Problem: JSON response wrapping >>>>> Currently CXF-RS is configured to wrap the JSON response into another >>>>> object. >>>>> >>>>> Example: Method signature: public List<AppointmentDTO> range(...) { >>>>> ... } (Example taken from >>>>> https://github.com/apache/openmeetings/blob/master/openmeetings-webservice/src/main/java/org/apache/openmeetings/webservice/CalendarWebService.java#L111 >>>>> ) >>>>> >>>>> OLD/CURRENT JSON Response: >>>>> { >>>>> "appointmentDTO": >>>>> [ >>>>> { >>>>> itemXYZ: 123, ... >>>>> } >>>>> ] >>>>> } >>>>> >>>>> >>>>> Proposed NEW/UPDATED JSON Response: >>>>> // no wrapping object around it, just return list >>>>> [ >>>>> { >>>>> itemXYZ: 123, ... >>>>> } >>>>> ] >>>>> >>>>> Reasoning: The wrapping "{ "appointmentDTO": ... }" should be dropped >>>>> from the json response body. "appointmentDTO" is generated but it is not >>>>> in >>>>> any schema definition or method signature. Cause there is nothing in the >>>>> method signature that would tell anybody where " "appointmentDTO": [" is >>>>> coming from. Other than by testing the API call and finding out by try and >>>>> error. >>>>> >>>>> CXF-RS allows configuring our Web Service to NOT generate that >>>>> wrapping element. And turn this behaviour off and just generate the list. >>>>> See "dropRootName" in the CXF docs at: >>>>> >>>>> https://cxf.apache.org/docs/jax-rs-data-bindings.html#JAXRSDataBindings-WrappingandUnwrappingJSONsequences >>>>> >>>>> *This affects all methods returning a JSON response body (which is >>>>> pretty much every API Method)* >>>>> >>>>> Please reply to this email if you have concerns, questions or >>>>> objections. >>>>> >>>>> Thanks! >>>>> Seb >>>>> >>>>> Sebastian Wagner >>>>> Director Arrakeen Solutions, OM-Hosting.com >>>>> http://arrakeen-solutions.co.nz/ >>>>> https://om-hosting.com - Cloud & Server Hosting for HTML5 >>>>> Video-Conferencing OpenMeetings >>>>> >>>>> <https://www.youracclaim.com/badges/da4e8828-743d-4968-af6f-49033f10d60a/public_url> >>>>> <https://www.youracclaim.com/badges/b7e709c6-aa87-4b02-9faf-099038475e36/public_url> >>>>> >>>>> >> >> -- >> Best regards, >> Maxim >> >> > > -- > Best regards, > Maxim > > -- Best regards, Maxim
