[
https://issues.apache.org/jira/browse/CASSANDRA-20404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17944898#comment-17944898
]
Maulin Vasavada commented on CASSANDRA-20404:
---------------------------------------------
Okay, [~smiklosovic] I think I fixed all the issues. I ran all the tests that
were failing locally prior and now its working. Made few changes. Main change
beyond just going back to giving out un-applied encryption options, was the bug
that in ServerEncryptionOptions constructor we started passing 'enabled' flag
which was not getting passed before (e.g. on the trunk) so that broke one of
the tests in EncryptionOptionsTest but now its working.
Can you please rerun the CI and validate?
> Improve EncryptionOptions class's `with` methods using Builder pattern
> ----------------------------------------------------------------------
>
> Key: CASSANDRA-20404
> URL: https://issues.apache.org/jira/browse/CASSANDRA-20404
> Project: Apache Cassandra
> Issue Type: Improvement
> Components: Legacy/Core
> Reporter: Maulin Vasavada
> Assignee: Maulin Vasavada
> Priority: Normal
> Fix For: 5.x
>
> Time Spent: 2h 50m
> Remaining Estimate: 0h
>
> Currently,
> [EncryptionOptions.java|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/config/EncryptionOptions.java#L520]
> allows constructing the object using `with` prefix method pattern which
> sounds similar to the Builder Pattern
> ([example)|https://blogs.oracle.com/javamagazine/post/exploring-joshua-blochs-builder-design-pattern-in-java]
> However, when there is any new parameter introduced in the class for a
> corresponding configuration option, it requires changes to all the
> constructor calls from those `with` methods to reflect the new parameter.
> This is error prone given so many string parameters and considering some
> [special
> use-cases|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/config/EncryptionOptions.java#L1190]
> it makes it challenging to modify the code for a simple task - adding a new
> parameter in the class.
> Proposal here is to use the Builder pattern (like shows in the example linked
> above) to improve the code in EncryptionOptions.java file.
> There could be a potential to consider using "java record" type that got
> introduced in Java 14+ or using [Lombok plugin|https://projectlombok.org/]
> but in order to be backward compatible and avoid additional dependency on
> external packages- may be we can simply follow the builder pattern.
> One change that we have to be careful about is - the new builder pattern
> implementation may not call `applyConfig()` everytime upon calling `with`
> methods compared to the current implementation. Only when final
> `Builder#build()` happens it will call the applyConfig() method . While that
> saves multiple initialization that happen today in Tests due to heavy usage,
> we have to assess if there is any impact due to this behavior change.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
