Skip to content

Fix serialization of DatabaseChangeLog with 'preConditions' for yaml #6118

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 6 commits into from
Aug 7, 2024
Merged

Fix serialization of DatabaseChangeLog with 'preConditions' for yaml #6118

merged 6 commits into from
Aug 7, 2024

Conversation

MalloD12
Copy link
Collaborator

@MalloD12 MalloD12 commented Jul 22, 2024 •
edited
Loading

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

Fixes #4379

When serializing a changelog like the one below:

databaseChangeLog:
- changeSet:
    id: '1'
    author: test
    objectQuotingStrategy: LEGACY
    preConditions:
    - or:
      - sqlCheck:
          expectedResult: 0
          sql: select count(*) from team
      - not:
        - sqlCheck:
            expectedResult: 0
            sql: select count(*) from referenced_table
      onError: HALT
      onFail: HALT
      onFailMessage: Some message
      onSqlOutput: IGNORE

it was producing a changelog like the one below:

databaseChangeLog:
- changeSet:
    id: '1'
    author: test
    objectQuotingStrategy: LEGACY
    preconditions:
      preConditions:
        nestedPreconditions:
        - or:
            nestedPreconditions:
            - sqlCheck:
                expectedResult: '0'
                sql: select count(*) from team
            - not:
                nestedPreconditions:
                - sqlCheck:
                    expectedResult: '0'
                    sql: select count(*) from referenced_table
        onError: !!liquibase.precondition.core.PreconditionContainer$ErrorOption 'HALT'
        onFail: !!liquibase.precondition.core.PreconditionContainer$FailOption 'HALT'
        onFailMessage: Some message
        onSqlOutput: !!liquibase.precondition.core.PreconditionContainer$OnSqlOutputOption 'IGNORE'

Class names have been removed from the onError, onFail, etc. Also, the required support to recognize and process correctly preconditions and nestedPreconditions node have been added.

Things to be aware of

Things to worry about

  • Yaml changelog format is behaving well with implemented changes. However, we need to ensure this is not breaking other formats (especially XML because JSON is based on YAML).

Additional Context

@MalloD12 MalloD12 self-assigned this Jul 22, 2024
@MalloD12 MalloD12 requested a review from filipelautert as a code owner July 22, 2024 19:10
@filipelautert filipelautert changed the title Fix issue 4379 Fix serialization of DatabaseChangeLog doesn't produce correct result with 'preConditions' for yaml Jul 23, 2024
@MalloD12 MalloD12 requested a review from tati-qalified July 23, 2024 14:29
@MalloD12 MalloD12 changed the title Fix serialization of DatabaseChangeLog doesn't produce correct result with 'preConditions' for yaml Fix serialization of DatabaseChangeLog with 'preConditions' for yaml Jul 23, 2024
@MalloD12
Copy link
Collaborator Author

Hey @mariochampion,

Today we were discussing about this changes made here with @tati-qalified and we realized now for YAML format (as well as for JSON) we are accepting two different formats on how to write preconditions on a changelog:

  preConditions:
        - or:
            - sqlCheck:
                expectedResult: 0
                sql: select count(*) from team
            - not:
                - sqlCheck:
                    expectedResult: 0
                    sql: select count(*) from referenced_table
          onError: HALT
          onFail: HALT
          onFailMessage: Some message
          onSqlOutput: IGNORE

and the other

preconditions:
      preConditions:
        nestedPreconditions:
        - or:
            nestedPreconditions:
            - sqlCheck:
                expectedResult: '0'
                sql: select count(*) from team
            - not:
                nestedPreconditions:
                - sqlCheck:
                    expectedResult: '0'
                    sql: select count(*) from referenced_table
        onError: HALT
        onFail: HALT
        onFailMessage: Some message
        onSqlOutput: IGNORE

The first one is the "original" and the second option is the "new" one after implementing these changes. I wanted to double-check with you whether this is acceptable or not. I think it's good we allow both to keep backward compatibility, but at the same time I think this could be a bit confusing for users. We said we will document this approach in the docs, but I would like to know what do you think about it.

cc: @lbadam @filipelautert

Thanks,
Daniel.

@adrian-velonis1
Copy link
Contributor

[DOC INTERNAL] https://datical.atlassian.net/browse/PD-4954

@filipelautert filipelautert merged commit 0468ec6 into master Aug 7, 2024
1 check passed
@filipelautert filipelautert deleted the fix-issue-4379 branch August 7, 2024 20:09
@filipelautert filipelautert added this to the 1NEXT milestone Aug 7, 2024
filipelautert pushed a commit that referenced this pull request Aug 13, 2024
@MalloD12 @filipelautert
Fix serialization of DatabaseChangeLog with 'preConditions' for yaml (#…
85a95e1 
…6118)

* YamlSerializer changes needed for processing preconditions and its nested preconditions correctly.

* Removed un-needed line from toMap method.

* Removed unneeded import, plus remove some if/else idententations updated.

* Preconditions support added to recognized and process preconditions and nestedPreconditions nodes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@filipelautert filipelautert filipelautert approved these changes

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

Assignees

@MalloD12 MalloD12

Labels
runFunctional&HarnessTests TypeBug
Projects
Liquibase Open Source
Status: Done
Milestone
v4.30.0
Development

Successfully merging this pull request may close these issues.

Serialization of DatabaseChangeLog doesn't produce correct result with 'preConditions'
4 participants
@MalloD12 @adrian-velonis1 @filipelautert @tati-qalified