-
-
Notifications
You must be signed in to change notification settings - Fork 189
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
Add support for custom JdbcCustomization via Spring Boot #357
Conversation
@kagkarlsson can you please review this PR. The bug is blocking us o:-) |
Currently on vacation for 1w, so cant look at it atm |
Sure this will not break for schemas of the old type? |
d1c18a3
to
0afdd57
Compare
I just tested - this change is not backward compatible. The question is if anybody is using task_data column in mssql, because from what I can see it cannot work currently. |
The problem seems to occur for MSSQL 2019+ |
p.setObject(3, serializer.serialize(taskInstance.getData())); | ||
p.setBytes(3, serializer.serialize(taskInstance.getData())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will not work without this change?
0afdd57
to
fc04e75
Compare
db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/MssqlJdbcCustomization.java
Outdated
Show resolved
Hide resolved
default byte[] getBytes(ResultSet rs, String columnName) throws SQLException { | ||
return rs.getBytes(columnName); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff moving the getBytes(..)
into the Customization
...heduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/AutodetectJdbcCustomization.java
Outdated
Show resolved
Hide resolved
|
||
private static final long serialVersionUID = 6577185347687546367L; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why, but I receive InvalidClassException, if this ID is missing and database entry was created with master version. I used the same JVM for compiling master and this branch, but still the error occurs - not sure why.
Caused by: java.io.InvalidClassException: com.github.kagkarlsson.scheduler.task.schedule.CronSchedule; local class incompatible: stream classdesc serialVersionUID = 6577185347687546367, local class serialVersionUID = 6577185347687514335
at java.base/java.io.ObjectStreamClass.initNonProxy(ObjectStreamClass.java:597) ~[na:na]
at java.base/java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:2051) ~[na:na]
at java.base/java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1898) ~[na:na]
at java.base/java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2224) ~[na:na]
at java.base/java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1733) ~[na:na]
at java.base/java.io.ObjectInputStream$FieldValues.<init>(ObjectInputStream.java:2606) ~[na:na]
at java.base/java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2457) ~[na:na]
at java.base/java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2257) ~[na:na]
at java.base/java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1733) ~[na:na]
at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:509) ~[na:na]
at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:467) ~[na:na]
at com.github.kagkarlsson.scheduler.boot.autoconfigure.DbSchedulerAutoConfiguration$2.deserialize(DbSchedulerAutoConfiguration.java:232) ~[db-scheduler-spring-boot-starter-master-SNAPSHOT.jar:na]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that's a bit strange. When I think about it, I suppose the serialVersionUID
really should be set in db-scheduler for everything that might be serialized... but if we set it it might be a breaking change, and we probably should have some idea of what jvm we are targeting (since I assume it will vary between versions)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to investigate that a bit more. We cannot add that without reason since it might break serialization for other users. That said, it should really have a serialVersionUID
set..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a mistake on my part not setting a value for serialVersionUID
for this class. Serialization will break for anyone using this class when we set it. The solution will be to migrate away from the class temporarily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a mistake on my part not setting a value for serialVersionUID
for this class. Serialization will break for anyone using this class when we set it. The solution will be to migrate away from the class temporarily.
fc04e75
to
b3e3282
Compare
...cheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/MssqlJtdsJdbcCustomization.java
Outdated
Show resolved
Hide resolved
I pushed some changes to your branch enabling configuring the @Bean
DbSchedulerCustomizer customizer() {
.... How does that sound? |
I will likely merge this PR, but remove |
Seems like your commit does the trick and there is no need for my code changes in this repo - my commit might be dropped. Or do I miss something? |
Brief, plain english overview of your changes here
Column task_data uses correct data type (for binary data) in MSSQL according to MS documentation
Fixes
#354
Reminders
cc @kagkarlsson