On 9/22/21 3:24 PM, Maxim Solodovnik wrote:


On Wed, 22 Sept 2021 at 19:20, Ali Alhaidary <ali.alhaid...@the5stars.org <mailto: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 :)))
A job very well done, thank you. We go through the testing with almost every build as well.

    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 :((
Yes, this is a blocker issue. I second you to look at old design issues and leave API for several years ;-)


    On Wed, 22 Sept 2021 at 19:09, Ali Alhaidary
    <ali.alhaid...@the5stars.org
    <mailto: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
        <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
        <https://localhost:5443/openmeetings> to
        https://localhost:5443/openmeetings/v1
        <https://localhost:5443/openmeetings/v1>

        On Wed, 22 Sept 2021 at 13:14, seba.wag...@gmail.com
        <mailto:seba.wag...@gmail.com> <seba.wag...@gmail.com
        <mailto:seba.wag...@gmail.com>> wrote:

            @Ali Alhaidary <mailto: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/
            <http://arrakeen-solutions.co.nz/>
            https://om-hosting.com <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
            <mailto: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
                <mailto: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/
                <http://arrakeen-solutions.co.nz/>
                https://om-hosting.com <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
                <mailto: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
                    <mailto:seba.wag...@gmail.com> wrote:
                    Hi,

                    as discussed in the comments section in
                    
https://github.com/apache/openmeetings/commit/4daf7c1f53738cd786dc976114cc5278b4f05f4f#comments
                    
<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
                    
<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
                    
<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/
                    <http://arrakeen-solutions.co.nz/>
                    https://om-hosting.com
                    <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

Reply via email to