Skip to content

Don't ignore logicalFilePath when including changelogs #6664

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jan 31, 2025
Merged

Don't ignore logicalFilePath when including changelogs #6664

merged 5 commits into from
Jan 31, 2025

Conversation

filipelautert
Copy link
Collaborator

Impact

  • Bug fix (non-breaking change which fixes expected existing functionality)
  • Enhancement/New feature (adds functionality without impacting existing logic)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

Don't ignore logicalFilePath & parent tree cascading when including changelogs.

Things to be aware of

Right now this kind of including fails:

parent, using logicalFilePath = bla
    child1 -> with change named = change, logicalFilePath = child1
    child2 -> with change named = change, logicalFilePath = child2

As it produces th following changeset id, ignoring the child logicalFilePath:

bla:change
bla:change

This fix makes it produces the expected changeset ids:

child1:change
child2:change

This fix also accounts for more levels, so you can have:

parent, using logicalFilePath = bla
     mid, using logicalFilePath = mid
       child1 -> with change named = change, logicalFilePath = child1
       child2 -> with change named = change, logicalFilePath = child2
    mid2, no logicalFilePath
       child3 -> with change named = change, no logicalPath

It produces:

mid:child1
mid:child2
bla:child3  (we skipped mid2 level but found bla on top)

Additional Context

Fixes #6656

@filipelautert filipelautert self-assigned this Jan 21, 2025
@wwillard7800 wwillard7800 self-requested a review January 21, 2025 21:33
wwillard7800
wwillard7800 approved these changes Jan 21, 2025
View reviewed changes
Copy link
Contributor

@wwillard7800 wwillard7800 left a comment

Choose a reason for hiding this comment

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

Good fix.

@filipelautert filipelautert mentioned this pull request Jan 22, 2025
Merged
3 tasks
@MalloD12 MalloD12 self-assigned this Jan 23, 2025
@MalloD12 MalloD12 removed their assignment Jan 23, 2025
@filipelautert filipelautert force-pushed the fix-logicalfilepath-cascading branch from 5fdffb0 to d5f3f6b Compare January 24, 2025 12:58
tati-qalified
tati-qalified approved these changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@tati-qalified tati-qalified left a comment

Choose a reason for hiding this comment

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

Tested manually with spring boot and with the CLI. Works as expected.

@filipelautert filipelautert force-pushed the fix-logicalfilepath-cascading branch from d5f3f6b to bb62f7f Compare January 31, 2025 13:22
@filipelautert filipelautert added this to the 1NEXT milestone Jan 31, 2025
@filipelautert filipelautert changed the title chore: don't ignore logicalFilePath & parent tree cascading when including changelogs. Don't ignore logicalFilePath when including changelogs Jan 31, 2025
@filipelautert filipelautert merged commit 6c524fc into master Jan 31, 2025
46 of 48 checks passed
@filipelautert filipelautert deleted the fix-logicalfilepath-cascading branch January 31, 2025 13:59
@GioPat
Copy link

GioPat commented Feb 4, 2025 •
edited
Loading

Be careful because if you pull the default docker image liquibase/liquibase the image or you install from CHOCO (on windows) the versions have this bad issue

filipelautert added a commit that referenced this pull request Feb 6, 2025
@filipelautert @MalloD12
Don't ignore logicalFilePath when including changelogs (#6664)
71a937e 
* chore: don't ignore logicalFilePath & parent tree cascading when including changelogs.

* fix: use raw value

* fix: do not overwrite previous result

* chore: refactoring

* Integration test added to validate changeset identifiers use the right logicalFilePath

---------

Co-authored-by: Daniel Mallorga <dmallorga@liquibase.com>
@vbartacek vbartacek mentioned this pull request Feb 7, 2025
Closed
2 tasks
filipelautert added a commit that referenced this pull request Feb 13, 2025
@filipelautert @MalloD12
Don't ignore logicalFilePath when including changelogs (#6664)
05f1d40 
* chore: don't ignore logicalFilePath & parent tree cascading when including changelogs.

* fix: use raw value

* fix: do not overwrite previous result

* chore: refactoring

* Integration test added to validate changeset identifiers use the right logicalFilePath

---------

Co-authored-by: Daniel Mallorga <dmallorga@liquibase.com>
(cherry picked from commit 6c524fc)
eichingertim pushed a commit to eichingertim/liquibase that referenced this pull request May 10, 2025
@filipelautert @MalloD12 @eichingertim
Don't ignore logicalFilePath when including changelogs (liquibase#6664)
c23ddee 
* chore: don't ignore logicalFilePath & parent tree cascading when including changelogs.

* fix: use raw value

* fix: do not overwrite previous result

* chore: refactoring

* Integration test added to validate changeset identifiers use the right logicalFilePath

---------

Co-authored-by: Daniel Mallorga <dmallorga@liquibase.com>
@rphegge rphegge mentioned this pull request Aug 25, 2025
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wwillard7800 wwillard7800 wwillard7800 approved these changes

@rberezen rberezen rberezen approved these changes

@tati-qalified tati-qalified tati-qalified approved these changes

@MalloD12 MalloD12 Awaiting requested review from MalloD12

Assignees

@filipelautert filipelautert

Labels
build&TestPro manuallyTested TypeBug
Projects
Liquibase Open Source
Status: Done
Milestone
v4.32.0
Development

Successfully merging this pull request may close these issues.

v4.31.0 Wrong changeset duplicate identifier check
6 participants
@filipelautert @GioPat @wwillard7800 @rberezen @tati-qalified @MalloD12