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

Makes sure that precondition onErrorMessage and onFailMessage are logged when an error happens #5134

Merged

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

Currently onErrorMessage and onFailMessage are logged only if the log-level property is set to something higher than info. As they are custom error messages they should be logged everytime an error occurs.

Things to be aware of

Things to worry about

Additional Context

@filipelautert filipelautert self-assigned this Oct 27, 2023
@filipelautert filipelautert linked an issue Oct 27, 2023 that may be closed by this pull request
2 tasks
@sonarcloud
Copy link

sonarcloud bot commented Oct 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

25.0% 25.0% Coverage
0.0% 0.0% Duplication

@rberezen
Copy link
Contributor

@tati-qalified could you validate the fix please? TIA!

@tati-qalified
Copy link
Contributor

It's working for me, with no issues.
I tested this using the CLI and a simple changelog:

databaseChangeLog:
  - preConditions:
    - onError: HALT
    - onFail: HALT
    - onErrorMessage: ---- this is an error ----
    - onFailMessage: ---- this is a failure ----
    - sqlCheck:
          expectedResult: 1
          sql: SELECT 1 from myTable
  -  changeSet:  
      id:  1  
      author:  nvoxland  
      preConditions:
        - onError: HALT
        - onFail: HALT
        - onErrorMessage:  ---- this is an error ----
        - onFailMessage: ---- this is a failure ----
        - foreignKeyConstraintExists:
            foreignKeyName: columna12345
            foreignKeyTableName: json_table
      changes:  
        -  createTable:  
            tableName:  newtable  
            columns:  
              -  column:  
                  name:  id  
                  type:  int  
                  autoIncrement:  true  
                  constraints:  
                    primaryKey:  true  
                    nullable:  false  
              -  column:  
                  name:  firstname  
                  type:  varchar(50)  
              -  column:  
                  name:  lastname  
                  type:  varchar(50)  
                  constraints:  
                    nullable:  false  
              -  column:  
                  name:  state  
                  type:  char(2)  
  • First I tested only the preconditions for the entire changelog. I commented out the preconditions inside the changeset.
  • After that, I uncommented the changeset preconditions and commented the changelog preconditions.
  • Then I used both the changelog and changeset preconditions.

For all three scenarios I followed these steps:

  1. Use a precondition that will generate an error
  2. Run liquibase update
  3. Run liquibase update --log-level= with each log level value
  4. Do the same with a precondition that will fail
  5. Run liquibase update with a precondition that will work - no errors nor failures

I tested around 6 different preconditions to be sure it worked in general. I don't believe it's necessary to test each one, though I can do it if someone thinks I should.

@tati-qalified
Copy link
Contributor

Apart from that, there's an error message that isn't too user-friendly:

When trying to run a changelog with an empty sqlCheck

databaseChangeLog:
  - preConditions:
    - onError: HALT
    - onFail: HALT
    - onErrorMessage: ---- this is an error ----
    - onFailMessage: ---- this is a failure ----
    - sqlCheck:

The error message is:

Unexpected error running Liquibase: Migration failed for changeset changelog.yml::1::nvoxland:
     Reason: java.lang.NullPointerException: Cannot invoke "String.replaceFirst(String, String)" because the return value of "liquibase.precondition.core.SqlPrecondition.getSql()" is null

It should probably say something along the lines of "sqlCheck attributes sql and expectedResult are required"

@filipelautert filipelautert merged commit cdaf85a into master Nov 3, 2023
35 of 43 checks passed
@filipelautert filipelautert deleted the 5067-preconditions-onerrormessage-does-not-work branch November 3, 2023 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Preconditions onErrorMessage does not work
3 participants