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

'contextFilter' is not allowed to appear in element 'includeAll', in contrast to the documentation #4479

Open
1 of 2 tasks
mattjudge opened this issue Jul 10, 2023 · 4 comments

Comments

@mattjudge
Copy link

Search first

  • I searched and no similar issues were found

Description

The docs for includeAll state that contextFilter can be included inside an includeAll tag, with the example of <includeAll path="files" contextFilter="DEV"/>, but following this results in the error

SEVERE [liquibase.integration] Error parsing line 8 column 57 of master.xml: cvc-complex-type.3.2.2: Attribute 'contextFilter' is not allowed to appear in element 'includeAll'.

Steps To Reproduce

Enter the following into your changelogfile.xml

<?xml version="1.0" encoding="UTF-8"?>   
<databaseChangeLog
xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.9.xsd">
<includeAll path="changelogs/" contextFilter="test"/>
</databaseChangeLog> 

Expected/Desired Behavior

The behaviour described in the docs.

Liquibase Version

4.23.0, 4.21.1

Database Vendor & Version

PostgreSQL

Liquibase Integration

CLI

Liquibase Extensions

No response

OS and/or Infrastructure Type/Provider

WSL

Additional Context

No response

Are you willing to submit a PR?

  • I'm willing to submit a PR (Thank you!)
@tati-qalified
Copy link
Contributor

Hello @mattjudge , thank you for letting us know about this issue.
I was able to recreate it, though I'm not sure if this is a bug that should be fixed or if the documentation should be changed. I will be forwarding this to the development team, though I would appreciate your opinion on this.
Thanks!
Tatiana

@mattjudge
Copy link
Author

mattjudge commented Jul 17, 2023

Thanks for confirming the issue! Since labels can be specified in includeAll tags (although I haven't tested this), it would seem to make sense to allow contexts to be specified as well.

@MalloD12
Copy link
Contributor

Hi @mattjudge,

Looking at the changelog provided in the "steps to reproduce" it shows you are using http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.9.xsd. Can you try using http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd instead?

Thanks,
Daniel.

@hohwille
Copy link

hohwille commented Feb 2, 2024

Hi there,
I have been using flyway before and started with liquibase and ran into the same issue.
However, I was not using XML but YAML.
IMHO this feature is kind of a requirement as otherwise it renders the includeAll useless in my use-case so I would consider this as a software bug rather than a documentation but.
What I am trying to archive is to have a db/changelog/test folder where I can add some *.sql files that only apply for testing:

databaseChangeLog:
   - includeAll:
       path: db/changelog/release/
   - includeAll:
       contexts: test
       path: db/changelog/test/

Testing a debugging liquibase here is very hard as IMHO the documentation and the logging needs some improvement:

  1. In case it is not working (executes all SQLs no matter what context the are assigned to) I have no clue if my contexts are not properly configured or if the change sets are not properly assigned to the according context. I would expect that liquibase should log the contexts that are actually activated:
14:26:21.534 [main] INFO  liquibase.lockservice - Successfully acquired change log lock
14:26:21.535 [main] INFO  liquibase.command - Using deploymentId: 6880381535
14:26:21.537 [main] INFO  liquibase.changelog - Reading from public.databasechangelog
Running Changeset: db/changelog/release/1.0/001-create-metadata.sql::create-metadata::joerg.hohwiller
14:26:21.584 [main] INFO  liquibase.changelog - Custom SQL executed
14:26:21.586 [main] INFO  liquibase.changelog - ChangeSet db/changelog/release/1.0/001-create-metadata.sql::create-metadata::joerg.hohwiller ran successfully in 28ms
Running Changeset: db/changelog/test/create-metadata-testdata.sql::create-metadata-testdata::joerg.hohwiller
14:26:21.601 [main] INFO  liquibase.changelog - Custom SQL executed
14:26:21.601 [main] INFO  liquibase.changelog - ChangeSet db/changelog/test/create-metadata-testdata.sql::create-metadata-testdata::joerg.hohwiller ran successfully in 9ms
14:26:21.607 [main] INFO  liquibase.util - UPDATE SUMMARY
14:26:21.607 [main] INFO  liquibase.util - Run:                          2
14:26:21.607 [main] INFO  liquibase.util - Previously run:               0
14:26:21.607 [main] INFO  liquibase.util - Filtered out:                 0
14:26:21.607 [main] INFO  liquibase.util - -------------------------------
14:26:21.607 [main] INFO  liquibase.util - Total change sets:            2
14:26:21.607 [main] INFO  liquibase.util - Update summary generated
14:26:21.608 [main] INFO  liquibase.command - Update command completed successfully.
Liquibase: Update has been successful. Rows affected: 9
14:26:21.612 [main] INFO  liquibase.lockservice - Successfully released change log lock
14:26:21.613 [main] INFO  liquibase.command - Command execution complete

So I would would add something like this at the beginning that would be very helpful:

14:26:21.534 [main] INFO  liquibase.xxx - The following contexts are configured: {test, dev}

Especially your documentation obviously only explains how to pass context config to the liquibase tool.
However, developers reading your documentation may use spring-boot or quarkus or whatever and may have to configure something like this:

liquibase.contexts=test
# or maybe this? - see https://stackoverflow.com/questions/40088915/using-spring-boot-profiles-with-liquibase-changeset-context-attribute-to-manage#comment99828335_44896267
spring.liquibase.contexts=test

If the only thing the developer sees is that all SQLs are executed no matter what you configure in changelog AND the application properties of the underlying framework, this is very frustrating as you have no clue how to narrow down the problem.
2. The documentation could be more precise and give examples:
https://docs.liquibase.com/change-types/includeall.html

Note: If you use Liquibase 4.23.0 or earlier, use the syntax contextFilter instead of context.

But in https://forum.liquibase.org/t/liquibase-contexts-is-not-working/7688 someone says this:

I figured it out. I think they changed “context” to “contextFilter” recently. The older version uses “context”.

So your users are left puzzled (should the documentation say later instead of earlier?).
Also it seems that every liquibase user stumbles over this problem:
https://stackoverflow.com/questions/30783353/why-are-all-contexts-executed-when-non-specified-on-update
Now your documentation at https://docs.liquibase.com/concepts/changelogs/attributes/contexts.html says this:

In Liquibase 4.23.0+, you can specify a context using @ in addition to AND, OR, !, and parentheses in the changesets. This requires you to designate a context or label explicitly when Liquibase is run for the changeset to run. The attribute context: @test causes a changeset to not run if Liquibase runs without any contexts provided.

When using this feature in YAML, you will discover that @ is a reserved character and you end up with this error:

Caused by: org.yaml.snakeyaml.scanner.ScannerException: while scanning for the next token
found character '@' that cannot start any token. (Do not use @ for indentation)
 in 'reader', line 5, column 18:
           contexts: @test

I am an experienced developer and quickly figured out that I have to quote the value as workaround but your documentation could be much more helpful and prevent your users from stepping into one pitfall after the next.
And even after quoting the value it was still executing the changeset/SQLs because of this github bug issue.
3. BTW: Good documentation has anchor IDs for all sections so I can refer to individual sections with deep-links to make my reference more precise (see e.g. stackoverflow link in comparison).
4. Your config is very lax. In general it is a great idea not to fail if a property in XML/JSON/YAML is unknown for down-/up-ward compatibility. Still it would be nice to get warnings logged for unknown properties so I can quickly see if I made typos. E.g.

14:26:21.534 [main] INFO  liquibase.xxx - Unknown property `inludeAll` in `db.changelog-master.yaml`

Developers will love such feedback that saves them from headaches :)

I hope my feedback will help to improve liquibase in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Discussion
Development

No branches or pull requests

5 participants