Switch resource handling from name to key

[marmoure]

Fixes #185
this PR switches to using key for resources in the DB manager for copying,renaming,navigation,creation
now creating a collection named AéB or with spaces in the name will work fine

This open source contribution to the eXide project was commissioned by the Office of the Historian, U.S. Department of State, https://history.state.gov/.

[joewiz]

@marmoure I confirmed that creating, deleting, copying/pasting, and cutting/pasting collections works, but I encountered errors with accessing created collections; renaming collections; and renaming, copying/pasting, and cutting/pasting resources. I found some solutions but others will need your help.

To reproduce the error accessing created collections:

1. Create /db/AéB with xmldb:create-collection("/db", "AéB")

2. In eXide's left sidebar in the "directory" tab, close and reopen the root /db collection. Notice that the "AéB" collection is shown.

3. In the same tab, open the /db/AéB subcollection. The operation silently fails. The Developer Tools > Network pane shows that the resulting call to the collections.xq http://localhost:8080/exist/apps/eXide/modules/collections.xq?root=/db/A%C3%A9B&view=r returns the following response:

   { "status" : "fail", "#text" : "Collection /db/AéB not found. Could not locate collection: /db/AéB" }

A fix for this is to add encodeURIComponent around the (sel.datum().key || "/db") in https://github.com/eXist-db/eXide/blob/develop/src/directory.js#L79.

To reproduce the error I observed renaming collections:

1. Using "File > Manage," select the "AéB" collection (created above), and select the "Rename" icon.
2. In the now-editable resource name field, add a "2" to the collection name, and hit the "Return" key.
3. This raises an error dialog, titled "Rename Error":

To reproduce the same issue with renaming resources, save the above query as AéB.xq, and repeat steps 2-4.

To reproduce the same issue with copying/pasting and cutting/pasting, use "File > Manage" to copy the AéB.xq query into the /db/AéB collection. The operation appears to succeed, but when toggling the /db/AéB collection closed and open using eXide's left sidebar in the "directory" tab, the operation fails. (My suggested fix to directory.js above fixes this by encoding the "key.")

Furthermore, when opening /db/AéB/AéB.xq in the editor pane, the path shown at the base of the editor pane is double-escaped: /db/A%C3%A9B/A%C3%A9B.xq. Here's a screenshot:

As a result, when clicking on the "arrow" icon to open the query in a new browser tab, the request returns a 404:

I think the code that sets the href="/exist/rest/db/A%C3%A9B.xq" and <span class="path">/db/A%C3%A9B.xq</span> shown in the screenshot above can be found at https://github.com/eXist-db/eXide/blob/develop/src/eXide.js#L961-L970. Unfortunately, since I don't know Javascript, I don't know how to further track down this problem.

[marmoure]

hi @joewiz i fixed the breadcrumbs part but the issue with the page not found is an exist issue
the rest client can't handle special characters in files name hopefully it will resolved soon
so this one can have a final test and get merged

[marmoure]

@joewiz there seems to be some failures which came as surprise to me, i ll try to replicate the build process and see if I can reproduce them

[marmoure]

@joewiz yes i confirm xmldb:store does not encode either the file name nor the collection

and xmldb:create-collection does encode the received collection name

[duncdrum]

This PR is mixing concerns, and WIP. I see no reason for the changes to the ci config or for removing existing tests. Please make a separate PR for these so we can discuss them separately.

[marmoure]

@duncdrum @joewiz this PR will be blocked till #498 is merged so we can have more details on the failures
the tests are working fine when tested locally with the same steps as CI but somehow it's failing, i m suspecting the deployment phase.

[marmoure]

According to this image

The changes introduced in this PR are not deployed correctly
@joewiz
@duncdrum

[joewiz]

I bet the stock version of eXide is being installed first before the version from this PR. We need a way either to prevent the stock version from being installed first or to install our version over the stock one. Packages are present in eXist's autodeploy are installed during eXist's startup; they are installed in alphanumeric order, but before installing a package eXist first checks if a package of the same package URI is already installed, and if one has already installed, the package being checked is skipped. So if the stock version of eXide included in the eXist Docker image's autodeploy folder is eXide-2.0.0.xar, and the version that CI copies into the autodeploy folder is eXide-3.0.0.xar, then v2.0.0 will be installed and v3.0.0 will be skipped. I think this is the problem preventing our CI tests from testing the PR's version of eXide.

[marmoure]

i ran a clean instance of the docker image and i checked the eXide version seems like it's "3.4.0"
@joewiz so the assumption exist isn't replacing exide because of old version it's not the case.

[marmoure]

i tried deploying exist using a different method which was recommended by @adamretter but unfortunately it didn't work
the method was sending a POST request to rest client which uploads then deploy the xar

#!/bin/bash

EXIST_REST_URI="http://localhost:8080/exist/rest"
EXIST_REST_USERNAME="admin"
EXIST_REST_PASSWORD=""

function upload_pkg_files() {
	local pkg_file_name="eXide-$1.xar"
	local pkg_file="./build/eXide-$1.xar"
	
	local curl_cmd=(curl --fail --user "${EXIST_REST_USERNAME}:${EXIST_REST_PASSWORD}" -X PUT -H "Content-Type: application/octet-stream" --data-binary "@${pkg_file}" "${EXIST_REST_URI}/db/${pkg_file_name}")

	echo "Uploading ${pkg_file_name} to eXist-db..."
		"${curl_cmd[@]}"
	echo "Uploaded OK."
}

function deploy_pkg_files() {
	local deploy_xq="<query xmlns=\"http://exist.sourceforge.net/NS/exist\">
						<text>
							let \$pkg-name := \"http://exist-db.org/apps/eXide\"
							let \$pkg-final-name := \"eXide-$1\"
							return
							(
								repo:undeploy(\$pkg-name),
								repo:install-and-deploy-from-db(\"/db/\" || \$pkg-final-name || \".xar\")
							)
						</text>
					</query>"

	local curl_cmd=(curl --fail --user "${EXIST_REST_USERNAME}:${EXIST_REST_PASSWORD}" -X POST -H "Content-Type: application/xml" -d "${deploy_xq}" "${EXIST_REST_URI}/db/")

	echo "Deploying ${pkg_file_name} to eXist-db..."
	"${curl_cmd[@]}"
	echo ""
	echo "Deployed OK."

}

upload_pkg_files $1
deploy_pkg_files $1

this script was invoked in the CI with the current version number as arg

[marmoure]

@joewiz after #500 is merged i ll rebase and run the tests again and hopefully it's a done deal

[joewiz]

@marmoure #500 is merged.

[joewiz]

@duncdrum Above you said:

This PR is mixing concerns, and WIP. I see no reason for the changes to the ci config or for removing existing tests. Please make a separate PR for these so we can discuss them separately.

The CI changes were moved into a separate PR: #500. I merged that PR provisionally, as it enables work on this PR to proceed. Your comments are certainly still welcome. Are there test scenarios you believe should be retained? Perhaps we can look into restoring them. The CI changes in #500 were needed at the very least as a sanity check, since eXist wasn't installing the eXide packages built from PRs, and this would affect any new PRs with tests. So once the automated testing works again, if anything needs to be put back we can take a look into it.

[marmoure]

@joewiz the deployment issue is no longer present but seems like we have some issues with the test runs it's flaky
so far i haven't been able to solve that as for the last run it was a login issue on one of the runs and that made everything fail

[marmoure]

@joewiz after adding multiple wait statements to the tests to slow down the runs it seems to be working as expected

[joewiz]

All tests pass locally and in CI.

[joewiz]

@marmoure Thank you very much for this thorough and well-tested PR! This clears up an entire class of bugs that affected users of eXide who used special characters in resource names (including users of TEI Publisher - see eeditiones/tei-publisher-app#48). We're on much more solid ground now, and the tests you added will flag any issues as we continue to investigate the related issues in the eXist core - eXist-db/exist#1824 and eXist-db/exist#3795.

CI issues were split into #498 and #500, and tests on the eXist release were re-added in the final commit here.

[joewiz]

p.s. A complementary CI fix was added to #503.