Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] util:document-id#1 throws an exception #4262

Closed
line-o opened this issue Mar 1, 2022 · 13 comments · Fixed by #4270
Closed

[BUG] util:document-id#1 throws an exception #4262

line-o opened this issue Mar 1, 2022 · 13 comments · Fixed by #4270
Labels
bug issue confirmed as bug high prio regression

Comments

@line-o
Copy link
Member

line-o commented Mar 1, 2022

Describe the bug

A simple call to

util:document-id("/db/apps/monex/index.html")

Throws an error Q{http://www.w3.org/2005/xqt-errors}FORG0001 "Invalid value for cast/constructor. can not convert '[I@40f342b9' to xs:int"

Expected behavior

An xs:int, the ID, if the resource exists or an empty sequence if the resource could not be found.

References

This was reported by gary.mansell@ricardo.com on exist-open
Might be related to #4167

To Reproduce

xquery version "3.1";

module namespace t="http://exist-db.org/xquery/test";

declare namespace test="http://exist-db.org/xquery/xqsuite";

declare
    %test:setUp
function t:setup() {
    let $test-collection := xmldb:create-collection("/db", "test")
    let $test-resource := xmldb:store($test-collection, "test.xml", "<root />")
    return ()
};

declare
    %test:tearDown
function t:tearDown() {
    xmldb:remove("/db/test")
};

declare
    %test:assertTrue
function t:test() {
    util:document-id("/db/test/test.xml") instance of xs:int
};

Context (please always complete the following information):

  • OS: macOS 10.15.7
  • eXist-db version: 6.0.1-SNAPSHOT
  • Java Version java8u312

Additional context

  • How is eXist-db installed? built from source
  • Any custom changes in e.g. conf.xml? none
@line-o line-o added bug issue confirmed as bug high prio regression labels Mar 1, 2022
@adamretter
Copy link
Member

Might be related to #4167

Shouldn't be. Generated id for a node is not the same as a document id.

If you could share the stack-trace then I can likely spot the issue quickly...

Regardless, in general I would advise anyone against using these functions that surface internal ids - as the ids are not stable.

@line-o
Copy link
Member Author

line-o commented Mar 1, 2022

Affected versions (so far)

  • 5.4.1
  • 6.0.0
  • 6.0.1

@garymansellricardo
Copy link

garymansellricardo commented Mar 1, 2022 via email

@line-o
Copy link
Member Author

line-o commented Mar 4, 2022

I expect this to work up to and including v5.3.1

@line-o
Copy link
Member Author

line-o commented Mar 7, 2022

@adamretter there is no stack trace. Just the dynamic XQuery error is thrown.

@line-o
Copy link
Member Author

line-o commented Mar 7, 2022

Why I believe that #4167 introduced this regression:
It changes the document id in exist-core/src/main/java/org/exist/dom/memtree/DocumentImpl.java

@garymansellricardo
Copy link

garymansellricardo commented Mar 9, 2022

I expect this to work up to and including v5.3.1

I can confirm that my query code runs OK on 5.3.1 and that this version does not suffer from the issue

@garymansellricardo
Copy link

Hi, I am slightly confused by the release versioning scheme. It seems to me that 4.10 (not 5.3.1) is the most recent version that I can run that does not exhibit the issue I have discovered - is that correct?

I notice that 5.3.1 suffers from log4j2 vulns that 4.10 seems to have addressed - so 4.10 would be the best and most recent version that I should run until this issue can be resolved in a new 6.x release (any ideas on when this might be?).

@line-o
Copy link
Member Author

line-o commented Mar 9, 2022

On Monday's community call we were not able to come to a conclusion how to address this issue.

Options are

  • drop the function
    This is a breaking change.
  • change the signature of the function to match the new return value
    This is also a breaking change.
  • convert the actual document id to an xs:int somehow
    Would limit the use of the returned id, as it is no longer suitable to retrieve documents with that ID.

Comments on what suits the people affected by this issue are welcome.

@garymansellricardo
Copy link

Well, as our app is broken if I can't run the query as is, then I vote for converting the doc id to an xs:int ;-)

I suspect that our app just uses the Doc id to loop through the xml tickets found in the DB, rather than pulling specific docs by ID, so think it should still work as long as they are returned as xs:ints

@line-o
Copy link
Member Author

line-o commented Mar 9, 2022

@garymansellricardo For your use-case you should consider switching to util:document-name instead.

@adamretter
Copy link
Member

adamretter commented Mar 10, 2022

convert the actual document id to an xs:int somehow

@line-o The document id's are java.lang.int... They can't possibly be anything else!

@line-o
Copy link
Member Author

line-o commented Mar 10, 2022

Good to know. I was not able to see that from the value in the error description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug issue confirmed as bug high prio regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants