Use ulid library safe for graalvm native compilation#817
Use ulid library safe for graalvm native compilation#817fjtirado merged 1 commit intoserverlessworkflow:mainfrom
Conversation
impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowApplication.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I think this should be more lazy.
Reason is that, with this implementaiton, if the user is replacing this factory, the secure ramdon and the ULID are still created, but never used.
There are two possibilities to achieve that:
- creare this class, as it is, (well, secureRamdon does not need to be a member in any case) in a lazy way in the WorkflowApplication (so create it only when the user has not set another WorkflowInsanceIdFactory)
- Leave WorkflowApplication as it is and create the ULID (and the SecureRamdon) when previousUlid is null.
There was a problem hiding this comment.
I now explain why I prefer the first one (lazy initialization of the factory in WorkflowApplication)
The get method can be simplified by creating previousUlid in the constructor, so you write a not synchronized get.
@Override
// note that you do not need to syncronized this
public String get() {
String result= currentUlid.toString();
currentUlid = ulid.nextMonotonicValue(currentUlid);
return result;
}
The constructor will look like
private final ULID ulid;
private ULID.Value currentUlid;
public MonotonicUlidWorkflowInstanceIdFactory () {
ulid = new ULID(new SecureRandom());
currentUlid = ulid.nextValue();
}
And in workflowapplicationbuilder.build you set the factory to MonoloictUlidWorkflowInstanceFactory if the id factory it is null (the user has not set its own one) rather than in builder initialization
There was a problem hiding this comment.
I now explain why I prefer the first one (lazy initialization of the factory in WorkflowApplication)
The get method can be simplified by creating previousUlid in the constructor, so you write a not synchronized get.
@Override
// note that you do not need to syncronized this
public String get() {
String result= currentUlid.toString();
currentUlid = ulid.nextMonotonicValue(currentUlid);
return result;
}
The constructor will look like
private final ULID ulid;
private ULID.Value currentUlid;
public MonotonicUlidWorkflowInstanceIdFactory () {
ulid = new ULID(new SecureRandom());
currentUlid = ulid.nextValue();
}
And in workflowapplicationbuilder.build you set the factory to MonoloictUlidWorkflowInstanceFactory if the id factory it is null (the user has not set its own one) rather than in builder initialization
Good catch!
@fjtirado, do we need to guarantee no duplicated keys in a thread-safe way? If not, I agree with you to remove the |
|
@mcruzdev Note: Im asuming nextMonotoicValue is thread-safe (it wont crash when invoked simoultaneuously from different threads), |
There was a problem hiding this comment.
@mcruzdev Remember that, in order to prevent SecureReandom instance to be created if the default id factory is overriden, this line (creation of the monotonic instance) neeeds to be moved to build() method
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
Many thanks for submitting your Pull Request ❤️!
What this PR does / why we need it:
com.github.f4b6a3:ulid-creatordependency.de.huxhorn.sulky:de.huxhorn.sulky.ulidfor ULID generation.WorkflowInstanceIdFactoryimplementation (UlidWorkflowInstanceIdFactory)Closes #812