Several improvements to GenericFilter's programmatic API #5083#5362
Several improvements to GenericFilter's programmatic API #5083#5362fractal3000 wants to merge 1 commit into
Conversation
…nfiguration() and refreshCurrentConfiguration() #5083 - MutableConfiguration: new interface extending Configuration with all mutating methods. RunTimeConfiguration now implements MutableConfiguration instead of Configuration, giving compile-time safety against calling mutating methods on a DesignTimeConfiguration. Mutating methods on Configuration are deprecated in favour of MutableConfiguration. - addAndSetCurrentConfiguration(): atomic alternative to calling addConfiguration() followed by setCurrentConfiguration(). The two-step sequence silently does nothing when the configuration has not yet been registered at the time setCurrentConfiguration is invoked. - refreshCurrentConfiguration(): stable public method to re-render the filter UI after programmatic modification of the current configuration. Was previously only accessible as the protected refreshCurrentConfigurationLayout(). - MutableConfiguration.protectedFromUserDeletion: flag that prevents the user from deleting a programmatic configuration through the UI. When true, GenericFilterRemoveAction hides the Remove button for that configuration, and GenericFilter.removeConfiguration() silently skips it.
3c398dc to
7564686
Compare
| import io.jmix.flowui.component.logicalfilter.LogicalFilterComponent; | ||
|
|
||
| import org.jspecify.annotations.Nullable; | ||
| import org.springframework.lang.Nullable; |
There was a problem hiding this comment.
We are using jspecify nullability annotations, spring nullable is deprecated
| * <p> | ||
| * {@link RunTimeConfiguration} implements this interface. {@link DesignTimeConfiguration} | ||
| * only implements the read-only {@link Configuration}. | ||
| * <p> | ||
| * Use this interface as the declared type when you need to call mutating methods, | ||
| * so the compiler catches misuse of {@link DesignTimeConfiguration} at compile time: | ||
| * <pre>{@code | ||
| * MutableConfiguration config = filter.runtimeConfigurationBuilder() | ||
| * .id("myConfig") | ||
| * .buildAndRegister(); | ||
| * config.setModified(true); | ||
| * }</pre> |
There was a problem hiding this comment.
Looks like a useless information, especially about implementations. Moreover, there is no runtimeConfigurationBuilder and such API on this moment.
| * @see Configuration | ||
| * @see RunTimeConfiguration | ||
| */ | ||
| @SuppressWarnings({"deprecation", "removal"}) |
There was a problem hiding this comment.
I think we shouldn't suppress this warnings on whole class
| public interface MutableConfiguration extends Configuration { | ||
|
|
||
| /** | ||
| * Returns whether the configuration is modified. |
There was a problem hiding this comment.
@returns should be used, the same for all classes
| @Deprecated(since = "3.0", forRemoval = true) | ||
| default boolean isAvailableForAllUsers() { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
This method shouldn't be deprecated because it used for calculating configuration name for all types of configuration
| * | ||
| * @param name a configuration name | ||
| * @throws UnsupportedOperationException when called on {@link DesignTimeConfiguration} | ||
| * @deprecated use {@link MutableConfiguration} instead |
There was a problem hiding this comment.
For use instead we should pass the appropriate methods in new interface e.g.:{@link MutableConfiguration#setName}
| if (configuration != getEmptyConfiguration() | ||
| && !(configuration instanceof DesignTimeConfiguration)) { | ||
| && !(configuration instanceof DesignTimeConfiguration) | ||
| && !(configuration instanceof MutableConfiguration mc && mc.isProtectedFromUserDeletion())) { |
There was a problem hiding this comment.
We shouldn't re-check this condition here. The runtime configuration should be available for internal removing, e.g. for the renaming process in the io.jmix.flowui.component.genericfilter.GenericFilterSupport#initFilterConfiguration. Otherwise the configuration will be duplicated.
Availablility of user-deletion should be handled (and already handled) in the GenericFilterRemoveAction isApplicable method.
| * {@link UnsupportedOperationException} when called on a {@link DesignTimeConfiguration}. | ||
| * Those methods are deprecated — use {@link MutableConfiguration} when you need to call them. | ||
| */ | ||
| public interface Configuration extends Comparable<Configuration> { |
There was a problem hiding this comment.
We have a lot of calls of all deprecated method. I think we should rewrite out own API if it is possible. Otherwise why do we introduce a new API if we are not using it.
Don't forget about backward compatibility.
| public void addAndSetCurrentConfiguration(Configuration configuration) { | ||
| addConfiguration(configuration); | ||
| setCurrentConfiguration(configuration); | ||
| } |
There was a problem hiding this comment.
Looks like a useless method. I think we should throw an exception or write a error log message when the user or programmer trying to set current configuration which does not exist.
#5083
MutableConfiguration: new interface extending Configuration with all mutating methods. RunTimeConfiguration now implements MutableConfiguration instead of Configuration, giving compile-time safety against calling mutating methods on a DesignTimeConfiguration. Mutating methods on Configuration are deprecated in favour of MutableConfiguration.
addAndSetCurrentConfiguration(): atomic alternative to calling addConfiguration() followed by setCurrentConfiguration(). The two-step sequence silently does nothing when the configuration has not yet been
registered at the time setCurrentConfiguration is invoked.
refreshCurrentConfiguration(): stable public method to re-render the filter UI after programmatic modification of the current configuration. Was previously only accessible as the protected refreshCurrentConfigurationLayout().
MutableConfiguration.protectedFromUserDeletion: flag that prevents the user from deleting a programmatic configuration through the UI. When true, GenericFilterRemoveAction hides the Remove button for that configuration,
and GenericFilter.removeConfiguration() silently skips it.