quartz-scheduler / quartz

Code for Quartz Scheduler
http://www.quartz-scheduler.org
Apache License 2.0
6.3k stars 1.94k forks source link

Fix #1095: 2.5 Check for MSSQLDelegate not full class name #1250

Closed melloware closed 2 weeks ago

melloware commented 2 weeks ago

Quarkus used and extended MSSQLDelegate so it skips this code: https://github.com/quarkusio/quarkus/blob/main/extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/jdbc/QuarkusMSSQLDelegate.java

So we need to check for the "MSSQLDelegate" instead of the full class name. Ideally we should check for instanceof but it looks like this was intentional here using the String and before the Delegate Class is loaded

Fix #1095 Fix #1101

cc @amergey

amergey commented 2 weeks ago

@melloware If I understand correctly, in quarkus DriverDelegate is not the one from quartz but implemented in quarkus. Your fix works for sure, but if someone would decide for some reasons to rename the quarkus class, he/she could break it again without knowing it.

This area of the code looks like a bit odd to me (not your current fix proposal, but the old code configuring specific sqlsqlserver). There are probably some emergencies for a fix here, this looks good to merge this PR.

Later, I would improve the code by delegating the sql lock execution to the DriverDelegate (has for other sql execution). The DB semaphore is tight to jdbc anyway. Requires some changes, but worth the effort, better maintainance, testability and less risk to break things

melloware commented 2 weeks ago

Agreed it does look suspicious here but has been like this for 10+ years! Yeah and I ideally it would check for instanceof MSSQLDelegate so any subclasses like our Quarkus one would be picked up.