[BUG] Variable declaration in library module is not processed reliably

[djbpitt]

To be able to better understand you problem, please add as much information as possible to this ticket. Always test your bugs against the latest stable release of exist. We cannot provide support for older versions here on GitHub. If the version of eXist that is experiencing the issue is more than 1 major version behind the most recent release, please consider posting a question on our mailing list.

Describe the bug
A clear and concise description of what the bug is.

The bug is reproducible in https://github.com/djbpitt/exist-import-test. Clone the repo, build, install, and load the index page in a browser. When you click on a link, the browser reports:

err:XPDY0002 undefined value for variable '$re:genres' [at line 16, column 37, source: String/-988904492058005134] In function: re:bgMsName(element(tei:TEI)) [31:35:String/-988904492058005134]

If you reload the page it responds correctly, and continues to do so. That is, the error is raised only the first time you try to use the resource. If you stop and restart eXist-db, the error will again appear on first use of the resource.

Expected behavior
A clear and concise description of what you expected to happen.

I expect the link to load the target result correctly on first access, and not only on subsequent access, as it does now.

Context (please always complete the following information)

• Build: eXist-6.2.0 (c8fa495)
• Java: 21.0.1 (Eclipse Adoptium)
• OS: Mac OS X 14.0 (aarch64)

I see the same behavior on my Centos 7 server. @line-o was able to reproduce the behavior with the current (as of 2023-10-28) development version of eXist-db.

Additional context

• How is eXist-db installed?

  • On MacOS with DMG. On Centos with JAR installer.

• Any custom changes in e.g. conf.xml?

  • No

Additional information

• Originally discussed in Slack at https://exist-db.slack.com/archives/CG2MRUZ35/p1698415596487759 beginning on 2023-10-27.
• If I replace the variable declaration for $re:genres with a let statement inside the function that uses that variable, there is no error. This is a workaround, rather than a solution, because 1) the variable should be available when declared in the prologue, and not only when declared in a let statement, and 2) in Real Life I use the same variable in more than one function, so declaring it in the prologue is DRYer than declaring it separately in each function that uses it.
• @line-o reports that: "For me adding util:log("info", $re:genres), to the function body weirdly does fix the issue on first request. A heisen-bug, the moment you debug it it is gone."

[adamretter]

@djbpitt I took a quick look at this, I note that it is using the XQuery URL Rewrite facility to forward requests for the URI read?filename=AM149NBW.xml to modules/read.xq?filename=AM149NBW.xml.

The XQuery URL Rewrite facility is a big part of what is in your repository, I wonder if we can narrow down the problem... if you don't use the rewriting but instead directly access read.xq, do you still get the same problem?, e.g. visiting:

http://localhost:8080/exist/rest/db/apps/exist-import-test/modules/read.xq?filename=AM149NBW.xml

[adamretter]

@djbpitt Not related to the issue you are reporting, but I have an observation on how you have structured your code and the ability to reuse your code. I might offer some advice...

This block in re-lib.xqm directly couples you to the use of the XQuery URL rewrite facility in eXist-db:

declare variable $re:genres as element(genre)+ :=
    doc(concat(
        request:get-attribute("$exist:root"), 
        request:get-attribute("$exist:controller"), 
        "/aux/genres.xml"
    ))/descendant::genre;

    declare function re:bgMsName($ms as element(tei:TEI)) as xs:string {
    ...

That should be avoided as it limits reuse and pollutes your code with the mechanisms employed by XQuery URL Rewrite (which really you should not have to worry about here).

If I understand correctly, you are trying to locate the data within your app package for use by your query?
If so, there are various mechanisms for doing this, the two most common that I employ are:

1. Determining this statically when I build the package. The path of where eXist-db will install the package is well-known to be one of two places depending on whether the package is of type application or library. For example, when I build my packages, I have my build tool inject the path into a config.xqm file within my code, e.g.:

   declare variable $config:data-collection-path as xs:string := "/db/system/repo/${package-final-name}/content/data";

   The ${packag-final-name} is injected in by the build system, in my case Maven.

2. Determining this dynamically at run-time. If I am using XQuery URL Rewrite then I place this in the controller.xq, else I place it in a separate XQuery Library Module:

   declare %private variable $config:module-load-path as xs:string := fn:replace(system:get-module-load-path(), "xmldb:exist://embedded-eXist-server/", "/");
   declare variable $config:data-collection-path as xs:string := $config:module-load-path || "/data";

Even once I have the $config:data-collection-path variable available, I still try and contain its use and avoid referencing it throughout my code. This prevents close-coupling of modules. Instead I prefer to pass it as a parameter to the functions that need it. For example:

declare function re:bgMsName($data-collection-path as xs:string, $ms as element(tei:TEI)) as xs:string {
...

or another second option:

declare function $re:genres-data($data-collection-path as xs:string) as element(genre)+ {
    fn:doc($data-collection-path as xs:string || "/aux/genres.xml")/descendant::genre
}

declare function re:bgMsName($genres as element(genre)+, $ms as element(tei:TEI)) as xs:string {
...

The advantage of this second option is that re:bgMsName is now testable as it is decoupled from the database storage, in fact it is now 100% standards compliant XQuery with no "side-effects", and as such you can call it with test data provided via the $genres parameter, and make expected assertions about what the results should be.

[djbpitt]

@adamretter Thank you for your generous interest and suggestions! I'll respond to the more general suggestions separately, but with respect to your question about possible interference from URL rewriting, I've pushed an update that does the following:

1. Hard-codes the location as /db/apps/exist-import-test/ instead of relying on controller variables.
2. Provides two links for each manuscript, one with URL rewriting and one without.

The original issue (variable is not defined on first call, but is available subsequently) happens with and without URL rewriting.

[djbpitt]

@adamretter I've now had a chance to read your suggestions about reducing my reliance on controller variables for locating resources, and I understand and appreciate the advantages of doing that. Thank you for the advice! The comments below are not your problem, to be sure, but in case they’re of any interest:

I understand the virtue, in the second method, of decoupling the function from dependence on the context. I don't, though, understand what system:get-module-load-path() is supposed to return. When I run it inside eXide (installed into the standard location) it returns "xmldb:exist://__new__2". I’ve obviously failed to understand something important because result does’t resemble something I recognize as able to get me to the location of eXide inside the database. Aside from not understanding what "__new__2" means, when I read the function documentation on the eXist-db site it seems to say that this function returns something points to a location on the file system, but I thought I wanted to point instead to a resource that had been installed into the database.

Meanwhile (and also not your problem), the first method is not within my abilities. I have no knowledge of build tools beyond the ability to type ant at the command line and let eXist-db build the app for me. I don't know anything more about how ant works and I don't know anything at all about how to use maven to build an eXist-db app. What I learned about building apps dates to a combination of https://exist-db.org/exist/apps/doc/development-starter.xml and your book, so I create pretty basic build.xml, expath-pkg.xml, and repo.xml files, type ant at the command line, and wait for the .xar file to be created. This has worked for me so far, although I understand that without more knowledge I won’t be able to go beyond the basics.

[adamretter]

@djbpitt Through a process of trial-and-error I have eliminated almost everything that is superfluous to causing your issue. I believe the issue is triggered through the use of fn:lang - see my PR here - djbpitt/exist-import-test#1

[djbpitt]

Thanks, @adamretter . I just merged your PR.

[adamretter]

I don't, though, understand what system:get-module-load-path() is supposed to return

It returns the database path to the XQuery Module that is currently being executed, i.e. the caller of that function.

When I run it inside eXide (installed into the standard location) it returns "xmldb:exist://__new__2"

In this instance, because the query you are executing from eXide is submitted to the server as a string and is not persisted in the database, there is no meaningful path that can be returned, so eXide is showing you something silly.
General good advice - Don't trust eXide!

[djbpitt]

General good advice - Don't trust eXide!

@adamretter Got it! I'll explore further …

[dizzzz]

Java21....?