handle empty lines at start of formatted sql

[smith-xyz]

Environment

Liquibase Version: 3.10.0

Liquibase Integration & Version: All

Liquibase Extension(s) & Version: N/A

Database Vendor & Version: N/A

Operating System Type & Version: All

Pull Request Type

• Bug fix (non-breaking change which fixes an issue.)
• Enhancement/New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Description

An issue spotted where empty space at the beginning of the formatted sql changelog would result in the changeset running as raw sql. This change is to find the first non empty string (or end of file) and perform the regex match to determine if the file is a formatted sql file so that the changeset is logged correctly. First time committing code here, thank you for any and all feedback.

Steps To Reproduce

1. Using a sql changelog file, include the following format where you have an empty first line (or many empty lines) before the '--liquibase formatted sql'

--liquibase formatted sql

--changeset test:1
create table test1 (
    id int IDENTITY,
    text varchar(255)
);
--rollback drop table test1;

1. Run the updateSql command to inspect the sql that will run.
2. Verify that the sql is marked to includeAll and that it will run as raw sql.
3. Verify that the change set marked for the change is incorrect. Should be for test:1 but instead it will be for raw:includeAll.

cf. Liquibase Forum Post

Actual Behavior

Before these changes, the changelog will run fine but save with raw:includeAll in the databasechangelog. The only issue with the changeset file is some whitespace at the beginning.

Expected/Desired Behavior

A simple check to find the first non empty line in the sql file, and then run the test to see if the text starts with 'liquibase formatted sql'. Otherwise run the file as raw sql storing the changeset id:author as raw:includeAll.

Screenshots (if appropriate)

screenshots on forum post:
cf. Liquibase Forum Post

Additional Context

Add any other context about the problem here.

Fast Track PR Acceptance Checklist:

<

!--- If you're unsure about any of these, just ask us in a comment. We're here to help|width=200,height=183!

-->

• Build is successful and all new and existing tests pass
• Added Unit Test(s)
• Added Integration Test(s)
• Added Test Harness Test(s)
• Documentation Updated

Test Requirements (Internal Liquibase QA)

• No new automated functional tests required; unit test is sufficient.

Manual Test Requirements

For the following tests, use the attached formatted SQL changelog.

Verify update-sql command returns the correct SQL to deploy changeset test:1.
Verify update is successful.
Verify DATABASECHANGELOG.ID=1-test1

Verify DATABASECHANGELOG.AUTHOR=bug_user.

---

Dev Handoff Notes (Internal Use)

Links

• Fixed Issue: Stand alone PR
• Local Branch: https://github.com/liquibase/liquibase/tree/smith-xyz-empty-line-issue-sql
• Unit Tests: https://github.com/liquibase/liquibase/pull/1713/checks
• Integration Tests: (shown in Unit Tests link above)
• Functional Tests: https://jenkins.datical.net/job/liquibase-pro/job/smith-xyz-empty-line-issue-sql/

Testing

• Steps to Reproduce: See above
• Guidance:
  • Impacts only formatted SQL

Dev Verification

Code review and automated tests

Need Help?

Come chat with us on our discord channel

┆Issue is synchronized with this Jiraserver Story by Unito

[codecov]

Codecov Report

Merging #1713 (24eb3a8) into master (99e76ca) will decrease coverage by 0.29%.
The diff coverage is 25.00%.

@@             Coverage Diff              @@
##             master    #1713      +/-   ##
============================================
- Coverage     48.20%   47.91%   -0.30%     
- Complexity     7558     7953     +395     
============================================
  Files           755      799      +44     
  Lines         36456    38973    +2517     
  Branches       6645     6983     +338     
============================================
+ Hits          17574    18672    +1098     
- Misses        16522    17798    +1276     
- Partials       2360     2503     +143     

Impacted Files	Coverage Δ	Complexity Δ
...core/formattedsql/FormattedSqlChangeLogParser.java	72.92% <25.00%> (-0.29%)	28.00 <0.00> (ø)
...ava/liquibase/statement/core/RawCallStatement.java	0.00% <0.00%> (-50.00%)	0.00% <0.00%> (-1.00%)
...n/java/liquibase/exception/LiquibaseException.java	57.14% <0.00%> (-42.86%)	4.00% <0.00%> (ø%)
...liquibase/integration/ant/AntResourceAccessor.java	75.00% <0.00%> (-25.00%)	3.00% <0.00%> (-2.00%)
...ava/liquibase/logging/core/AbstractLogService.java	42.85% <0.00%> (-23.81%)	1.00% <0.00%> (ø%)
.../integration/ant/type/ChangeLogParametersType.java	72.00% <0.00%> (-20.00%)	10.00% <0.00%> (-2.00%)
...in/java/liquibase/logging/core/JavaLogService.java	68.75% <0.00%> (-12.74%)	10.00% <0.00%> (ø%)
...base/statement/core/SetColumnRemarksStatement.java	87.50% <0.00%> (-12.50%)	7.00% <0.00%> (+1.00%)	⬇️
.../liquibase/integration/ant/DatabaseUpdateTask.java	40.00% <0.00%> (-12.00%)	2.00% <0.00%> (-1.00%)
...a/liquibase/changelog/visitor/RollbackVisitor.java	59.52% <0.00%> (-11.45%)	9.00% <0.00%> (+1.00%)	⬇️
... and 189 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99e76ca...24eb3a8. Read the comment docs.

[molivasdat]

Hi @smith-xyz Thanks for creating this PR for enhancement.

Here's what happens next:
A member of the Liquibase team will take a look at your contribution and may suggest:

• Changes based on guidelines for PR submission
• Additional tests (unit and/or integration) and/or Test Harness Tests
• Provide other feedback, like requesting changes.

The PR will be prioritized according to our internal development and testing capacity.

We’ll let you know when it’s ready to move to the next step or if any changes are needed.

[nvoxland]

Thanks for the fix!

[sync-by-unito]

➤ obovsunivskyi commented:

verified this on branch smithxyzemptylineissue-sql#5 and works fine. Moving to RTM.