[fix] disable global restXQ trigger #4699

line-o · 2023-01-24T00:56:49Z

Description:

This PR disables the restXQ trigger in collection.xconf.init. This is a breaking change as applications relying on this trigger to
be activated globally will have to add this trigger to their application's collection.xconf.

In my testing I was able to add two separate applications with both having the restXQ-trigger activated in their own
collection.xconf (both were based on https://gist.github.com/joewiz/28dd9b8454d14b4164a0). Modifying the code
showed up immediately when the exported routes were called with /exist/restxq/{route}.

Reference:

This trigger has led to several severe problems in the past with heap-memory exceptions when installing XAR-packages and
hanging databases. There is also an unsolved issue with modules with circular dependencies.

related [BUG] RESTXQ infinite loop with circular dependency #3448

Type of tests:

In order to replicate my test you can check out this PR, build it and then create one or more test-applications using the template below.

xquery version "3.1";

module namespace my-app = "http://my/app";

(: 
    Save this file into eXist anywhere (e.g., /db/restxq-template.xqm or 
    /db/apps/my-app/modules/restxq-template.xqm).
    
    Note that the collection configuration file where you store the module must invoke
    the restXQ trigger.

    Add a collection.xconf next to your script (or in any parent collection).
    It has to have the following trigger configuration (along with any other configurations).

        <collection xmlns="http://exist-db.org/collection-config/1.0">
            <triggers>
                <trigger class="org.exist.extensions.exquery.restxq.impl.RestXqTrigger"/>
            </triggers>
        </collection>
    
    run the handler with GET http://localhost:8080/exist/restxq/test
:)

declare namespace rest = "http://exquery.org/ns/restxq";
declare namespace output = "http://www.w3.org/2010/xslt-xquery-serialization";

declare
    %rest:GET
    %rest:path("/test")
    %output:media-type("text/html")
    %output:method("html5")
function my-app:hello() {
    let $title := 'Hello world!'
    return
        <html xmlns="http://www.w3.org/1999/xhtml">
            <head>
                <title>{$title}</title>
            </head>
            <body>
                <h1>{$title}</h1>
            </body>
        </html>
};

sonarcloud · 2023-01-24T01:23:50Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

adamretter · 2023-01-24T09:41:36Z

As I previously stated to you @line-o in the public eXist-db Slack, I will not accept this change

line-o · 2023-01-24T09:42:13Z

I know you wrote you were against it. As I was able to confirm in my testing, restxq is still an option for application developers as long as they set the trigger for the collections that contain actual handlers. So, I hope that I can convince users to just add it to where they need it. With the positive side effect for everyone that all other collections are no longer affected by the problems it introduces by being active everywhere.

line-o · 2023-01-24T09:53:45Z

Here you have the chance to argue why this is still needed or what I have not thought about. Just not accepting it is a little shallow of an argument.

adamretter · 2023-01-24T09:57:45Z

Would you like me to copy and paste my points from Slack?

line-o · 2023-01-24T10:11:29Z

Would you like me to copy and paste my points from Slack?

Yes, please do so.

reinhapa · 2023-01-30T19:31:42Z

As I previously stated to you @line-o in the public eXist-db Slack, I will not accept this change

Hi @adamretter,
I tried to find the discussion about this but was not able to find it anymore. Could you explain what the problem is in your opinion as I do not really seem understand the actual problem with this change.

dizzzz · 2023-02-06T21:07:28Z

@adamretter in exist index configuration for a collection is taken from the collection.xconf for that collection, or when missing the one from the most close by parent collection.xconf .
How does that work for triggers?

duncdrum · 2023-02-09T07:39:59Z

@line-o there is a test for docker using restxq. This will need to modified to either activate it by using a custom conf.xml for that temporary container; or disabled.

line-o · 2023-02-09T07:43:13Z

@duncdrum Yes I saw that. Maybe it helps to put in the extra effort to show that everything is still running fine afterwards and what needs to be adapted.

windauer · 2023-02-09T17:14:36Z

@adamretter in Slack you wrote that the people who are not using restxq should simply disable it. Problem with this approach is imho that endusers will not know whats the cause of their issues and that they should search for "disabling restxq". They will notice heavy RAM usage only due to installing a XAR (and eXist-db not freeing the RAM afterwards, even when you trigger a GC via Monex). Or that the installation of frus takes 40min with a default eXist-db while it would take only 20min on the same machine with restxq disabled. In worst case they even step away from eXist-db without knowing that their issue could have simply been solved by disabling the restxq trigger.

I would be all in for adding a "enable restxq" trigger to the app generator @duncdrum is maintaining and to add the information how and where to enable restxq prominent in the documentation but to disable it by default.

StephanMa · 2023-02-09T17:26:43Z

My two cents from an eXist user…
In almost every installation I am using restxq. Disable it from the beginning, is maybe no dealbreaker, but some extra step.

The argument of stepping away from exist… well I am using eXist for 8 years… Every time I have to work with it, I discover more and more bugs/mysterious problems/what so ever… we were used to problems…

line-o · 2023-02-27T11:52:31Z

@StephanMa are you using restxq - annotating functions to route http traffic - or the REST-like API of exist?
The easiest way to distinguish them is by the URLs you are calling

[/exist]/restxq/... -> restxq
[/exist]/rest/... -> REST-like API

This is about restxq and does not disable the REST-like API of exist.

StephanMa · 2023-02-27T11:53:45Z

-> [/exist]/restxq/... -> restxq!

duncdrum · 2023-04-07T19:41:24Z

See #4855 (comment)

adamretter · 2023-04-09T09:38:10Z

#4855 is unrelated to this!

[fix] disable global restXQ trigger

f5ea92c

line-o added the needs documentation Signals issues or PRs that will require an update to the documentation repo label Jan 24, 2023

line-o added this to the eXist-7.0.0 milestone Jan 24, 2023

line-o requested a review from a team January 24, 2023 11:32

adamretter mentioned this pull request Jul 28, 2023

Fixes to eXist-db regarding circular imports #4996

Merged

reinhapa closed this in #4996 Oct 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fix] disable global restXQ trigger #4699

[fix] disable global restXQ trigger #4699

line-o commented Jan 24, 2023 •

edited

sonarcloud bot commented Jan 24, 2023

adamretter commented Jan 24, 2023

line-o commented Jan 24, 2023 •

edited

line-o commented Jan 24, 2023

adamretter commented Jan 24, 2023 •

edited

line-o commented Jan 24, 2023

reinhapa commented Jan 30, 2023

dizzzz commented Feb 6, 2023

duncdrum commented Feb 9, 2023

line-o commented Feb 9, 2023 •

edited

windauer commented Feb 9, 2023

StephanMa commented Feb 9, 2023

line-o commented Feb 27, 2023 •

edited

StephanMa commented Feb 27, 2023 •

edited

duncdrum commented Apr 7, 2023

adamretter commented Apr 9, 2023

[fix] disable global restXQ trigger #4699

[fix] disable global restXQ trigger #4699

Conversation

line-o commented Jan 24, 2023 • edited

Description:

Reference:

Type of tests:

sonarcloud bot commented Jan 24, 2023

adamretter commented Jan 24, 2023

line-o commented Jan 24, 2023 • edited

line-o commented Jan 24, 2023

adamretter commented Jan 24, 2023 • edited

line-o commented Jan 24, 2023

reinhapa commented Jan 30, 2023

dizzzz commented Feb 6, 2023

duncdrum commented Feb 9, 2023

line-o commented Feb 9, 2023 • edited

windauer commented Feb 9, 2023

StephanMa commented Feb 9, 2023

line-o commented Feb 27, 2023 • edited

StephanMa commented Feb 27, 2023 • edited

duncdrum commented Apr 7, 2023

adamretter commented Apr 9, 2023

line-o commented Jan 24, 2023 •

edited

line-o commented Jan 24, 2023 •

edited

adamretter commented Jan 24, 2023 •

edited

line-o commented Feb 9, 2023 •

edited

line-o commented Feb 27, 2023 •

edited

StephanMa commented Feb 27, 2023 •

edited