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

Added parameterless ctor to the Serializers to allow load them from hazelcast standard config file. #453

Merged
merged 3 commits into from Feb 23, 2024

Conversation

MonDeveloper
Copy link
Contributor

@MonDeveloper MonDeveloper commented Feb 21, 2024

As suggested by @vladimir-bukhtoyarov during the discussion on the Issue #477 I'm submitting a PR to enable the loading of the Bucke4j custom serializers through the Hazelcast standard configuration file instead of the need to do it programmatically.

A quick disclaimer:
I'm not a skilled java guy and this is the first Spring-less project I'm working on since the University, and I was used to play the brand new "Legend of Zelda: Ocarina of time" at that times!

This PR includes:

  • a new static class SerializationUtility where I put all the logic to retrieve the typeIdBase based on System Properties or Environment Variables and compute all the typeId for the well-known Serializers based on the offsets declared statically
  • a new parameterless ctor in all ther three well-known Serializers to allow them being used in the hazelcast configuration file with just their name
  • a new JTest class to test the SerializationUtility helper
  • a new JTest class (cloned by the already existing HazelcastWithCustomSerializersTest with a different loading of the hazlecast server using the same hazelcast config primitives used by hazelcast when it loads from configuration file.
  • a new test dependency to help adding Env Var during tests

…azelcast standard config file.

Includes UnitTests to cover both the SerializationUtility atomic functionalities and the whole Hazelcast serialization part.
@MonDeveloper
Copy link
Contributor Author

I forgot to mention I also touched the already existing HazelcastProxyManager<K>.addCustomSerializers just to make it using the new SerializationUtility helper class so we put only there the well-known serializers typeId offsets instead of having them in the form of +n close to the typeIdBase.
The change is functionally invariant and it has been successfully tested.

@vladimir-bukhtoyarov
Copy link
Collaborator

Good. I need a time to test this request, especially for case of backward compatibility.

@vladimir-bukhtoyarov vladimir-bukhtoyarov changed the base branch from master to 8.9_standalone_hazelcast_serialization February 22, 2024 08:23
@vladimir-bukhtoyarov
Copy link
Collaborator

vladimir-bukhtoyarov commented Feb 22, 2024

It would be nice to add paragraph there https://github.com/bucket4j/bucket4j/blob/master/asciidoc/src/main/docs/asciidoc/distributed/jcache/hazelcast.adoc about of serialization configs in standalone cluster.

import java.util.function.Supplier;

public class SerializationUtilities {
public static final String TYPE_ID_BASE_PROP_NAME = "Bucket4jSerializers_TypeId_Base";
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer to rename to "bucket4j.hazelcast.serializer.type_id_base"

@vladimir-bukhtoyarov vladimir-bukhtoyarov changed the base branch from 8.9_standalone_hazelcast_serialization to 8.9 February 22, 2024 17:09
…roperly configure the custom serialization for a standalone cluster
@MonDeveloper
Copy link
Contributor Author

MonDeveloper commented Feb 23, 2024

Just pushed 2 commit in my branch:

  1. changed the parameter name according to the suggested value
  2. added a chapter into the asciidoc documentation

@vladimir-bukhtoyarov vladimir-bukhtoyarov merged commit 474d23f into bucket4j:8.9 Feb 23, 2024
@vladimir-bukhtoyarov
Copy link
Collaborator

vladimir-bukhtoyarov commented Feb 23, 2024

I am going to release this weekend.

@MonDeveloper
Copy link
Contributor Author

I am going to release this weekend.

Nice!
Thank you very much.

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

Successfully merging this pull request may close these issues.

None yet

2 participants