Skip to content

[fix] remove omit-original-xml-declaration flag#6277

Draft
line-o wants to merge 1 commit into
eXist-db:developfrom
line-o:remove-custom-xml-decl-flag
Draft

[fix] remove omit-original-xml-declaration flag#6277
line-o wants to merge 1 commit into
eXist-db:developfrom
line-o:remove-custom-xml-decl-flag

Conversation

@line-o
Copy link
Copy Markdown
Member

@line-o line-o commented Apr 29, 2026

Description:

The custom serialization flag omit-original-xml-declaration is removed. The only flag that controls if the xml declaration is present or not when serializing a document is the standard omit-xml-declaration.

If a document has a stored declaration in the database then it will be used. If there is none then the xml declaration is being created.

The only remaining issue is how to handle stored declarations in backups. They should be part of the backup if - and only if - there is one stored.

---- IMPORTANT ----

This PR was opened to start the discussion. I will add it to the agenda of next weeks community call.

So why is this an issue in the first place?
The newly added, custom serialization flag has precedence over the the standard flag that should control if an XML declaration is part of the output or not. This change in behaviour broke tests in node-exist and xst which is how I learned about it in the first place. I suspect that this feature might not even be documented yet.

I believe this extra flag does not need to be exposed. To discuss this, is the reason why I created this PR.


Reference:

added with 855922a

Type of tests:

none added so far

The custom serialization flag omit-original-xaml-declaration is removed.
The only flag that controls if the xml declaration is present when
serializing a document is the standard omit-xml-declaration.

If a document has a stored declaration in the database then it will be
used. If there is none then the xml declaration is being created.

The only remaining issue is how to handle stored declarations in
backups.
@line-o line-o requested a review from a team as a code owner April 29, 2026 14:55
Comment on lines 121 to +122
if (doc.getXmlDeclaration() != null){
if ("no".equals(getProperty(EXistOutputKeys.OMIT_ORIGINAL_XML_DECLARATION, "no"))) {
if ("no".equals(getProperty(OutputKeys.OMIT_XML_DECLARATION, "yes"))) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Join this if statements as suggested by Codacy

@duncdrum
Copy link
Copy Markdown
Contributor

duncdrum commented Apr 30, 2026

test failures

Error: core] [ERROR] Tests run: 8, Failures: 4, Errors: 0, Skipped: 0, Time elapsed: 19.33 s <<< FAILURE! -- in org.exist.backup.XMLDBBackupTest
Error: core] [ERROR] org.exist.backup.XMLDBBackupTest.backupRestoreWithXmlDecl[local (classic)] -- Time elapsed: 0.116 s <<< FAILURE!
java.lang.AssertionError: Content should contain XML declaration
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.exist.backup.XMLDBBackupTest.backupRestoreWithXmlDecl(XMLDBBackupTest.java:173)
Error: core] [ERROR] org.exist.backup.XMLDBBackupTest.backupRestoreWithXmlDecl[remote (classic)] -- Time elapsed: 3.974 s <<< FAILURE!
java.lang.AssertionError: Content should contain XML declaration
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.exist.backup.XMLDBBackupTest.backupRestoreWithXmlDecl(XMLDBBackupTest.java:173)
Error: core] [ERROR] org.exist.backup.XMLDBBackupTest.backupRestoreWithXmlDecl[local (dedup)] -- Time elapsed: 0.102 s <<< FAILURE!
java.lang.AssertionError: Content should contain XML declaration
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.exist.backup.XMLDBBackupTest.backupRestoreWithXmlDecl(XMLDBBackupTest.java:173)
Error: core] [ERROR] org.exist.backup.XMLDBBackupTest.backupRestoreWithXmlDecl[remote (dedup)] -- Time elapsed: 3.518 s <<< FAILURE!
java.lang.AssertionError: Content should contain XML declaration
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.exist.backup.XMLDBBackupTest.backupRestoreWithXmlDecl(XMLDBBackupTest.java:173)

@line-o
Copy link
Copy Markdown
Member Author

line-o commented Apr 30, 2026

@duncdrum yes this is expected until we come to a solution for this issue

The only remaining issue is how to handle stored declarations in backups. They should be part of the backup if - and only if - there is one stored.

I should have marked this as a draft perhaps.

@line-o line-o marked this pull request as draft April 30, 2026 09:16
@line-o line-o added this to v7.0.0 Apr 30, 2026
@line-o line-o added discuss ask for feedback regression labels Apr 30, 2026
@line-o line-o self-assigned this Apr 30, 2026
@line-o line-o moved this to In progress in v7.0.0 Apr 30, 2026
@line-o line-o moved this from In progress to Ready in v7.0.0 Apr 30, 2026
@line-o line-o moved this from Ready to Backlog in v7.0.0 Apr 30, 2026
@line-o
Copy link
Copy Markdown
Member Author

line-o commented May 18, 2026

The consensus when discusssing this topic on the community call on 18th May was to

  • remove the custom serialization flag entirely
  • when doing backups set "omit-xml-declaration" flag to false which will mean that all xml resources will have a xml declaration from then on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discuss ask for feedback regression

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

3 participants