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

Class cast exception in standard lock service #1878

Conversation

jocmer-evooq
Copy link
Contributor

@jocmer-evooq jocmer-evooq commented Jun 2, 2021

Environment

Liquibase Version:
4.3.1 -> 4.4.0

Liquibase Integration & Version: <Pick one: CLI, maven, gradle, spring boot, servlet, etc.>
At least spring boot, but all should be impacted

Liquibase Extension(s) & Version:

Database Vendor & Version:
MySQL with driver 8.0.23

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

The issue #1749 occurs when the locks are listed (after the lock times out). This fixes the cast issue. This PR also include a unit test to ensure it works with java.sql.Date and LocalDateTime.
To test it, it was not easy to mock the ExecutorService as the Scope does not allow to override a singleton. Thus I upgraded Mockito in order to use it's new mock static ability.

Steps To Reproduce

List the steps to reproduce the behavior.

  • Use a MySQL DB with driver 8.0.23
  • Create a Liquibase lock with a proper LOCKGRANTED date
  • Run the migration (it will be locked) wait for the timeout

Actual Behavior

A class cast exception is thrown (see #1749)

Expected/Desired Behavior

The locks are listed without any exception

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

Fixes #1749

Test Requirements (Internal Liquibase QA)

  • Verify all automated functional tests pass on Jenkins branch.
  • No new automated functional test required for this fix; the unit test is sufficient (and the proper level for a fix of this nature).

Manual Test Setup

  • On a MySQL database, UPDATE the DATABASECHANGELOGLOCk table to lock the DATABASECHANGELOG table:
UPDATE testdb1.DATABASECHANGELOGLOCK SET `LOCKED` = 1, LOCKEDBY = 'gemfire-PC (192.168.50.238)', LOCKGRANTED = '2021-06-10 12:15:14.553' WHERE ID = 1 AND `LOCKED` = 0;
  • Execute a Liquibase CLI update (which will hang, waiting on the lock to release).

CLI
Verify list-locks does not throw a stack trace.
Verify list-locks returns the lock information to the console.
Verify when the update times out due to the lock, there is no stack trace thrown.

Spring Boot (Start the Application while database is locked)

Verify list-locks does not throw a stack trace.
Verify list-locks returns the lock information to the console.

Dev Handoff Notes (Internal Use)

Links

Testing

  • Steps to Reproduce: Specified best in the PR
  • Guidance:
    • Changes only impact the list-locks command

Dev Verification

Ran the list-locks command pre-fix to see it fail, and post-fix to see it pass.
(Did not save output at the time)

┆Issue is synchronized with this Jira Story by Unito

Copy link
Contributor

@nvoxland nvoxland left a comment

Choose a reason for hiding this comment

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

Changes look good. Thanks!

@nvoxland nvoxland requested a review from suryaaki2 June 3, 2021 16:59
@nvoxland nvoxland changed the base branch from master to 4.4.x June 29, 2021 21:06
@sync-by-unito
Copy link

sync-by-unito bot commented Jul 20, 2021

➤ obovsunivskyi commented:

need to verify my test steps with Nathan Voxland during desk check.

@sync-by-unito
Copy link

sync-by-unito bot commented Jul 21, 2021

➤ obovsunivskyi commented:

Verified and works fine on build #4. Moving to RTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants