On Mon, 10 Oct 2022 12:07:38 GMT, Aleksei Efimov <aefi...@openjdk.org> wrote:
>> src/java.naming/share/classes/com/sun/naming/internal/ObjectFactoriesFilter.java
>> line 99:
>>
>>> 97: return globalResult == Status.ALLOWED;
>>> 98: }
>>> 99:
>>
>> If I'm not mistaken there's no point in checking the specific filter if the
>> global filter state is REJECTED. So instead of switching on the
>> specificResult below, maybe you should change the logic to switch on the
>> globalResult instead and only call the specific filter if needed?
>
>> If I'm not mistaken there's no point in checking the specific filter if the
>> global filter state is REJECTED. So instead of switching on the
>> specificResult below, maybe you should change the logic to switch on the
>> globalResult instead and only call the specific filter if needed?
>
> Yes - there is no point, and that will reduce number of `checkInput` calls on
> a specific filter, if it is not the global one. That's how it will look like:
>
> private static boolean checkInput(ConfiguredFilter filter, FactoryInfo
> serialClass) {
> var globalFilter = GLOBAL_FILTER.filter();
> var specificFilter = filter.filter();
> Status globalResult = globalFilter.checkInput(serialClass);
>
> // Check if a specific filter is the global one
> if (filter == GLOBAL_FILTER) {
> return globalResult == Status.ALLOWED;
> }
> return switch (globalResult) {
> case ALLOWED -> specificFilter.checkInput(serialClass) !=
> Status.REJECTED;
> case REJECTED -> false;
> case UNDECIDED -> specificFilter.checkInput(serialClass) ==
> Status.ALLOWED;
> };
> }
>
>
> The `if (filter == GLOBAL_FILTER) {` check can be also removed but without it
> the `checkInput` will be called twice on global filter for the case where
> `specific == global`, ie call from the `checkGlobalFilter`.
The code above LGTM. An alternative could be:
private static boolean checkInput(ConfiguredFilter filter, FactoryInfo
serialClass) {
var globalFilter = GLOBAL_FILTER.filter();
var specificFilter = filter.filter();
Status globalResult = globalFilter.checkInput(serialClass);
return switch (globalResult) {
case ALLOWED -> filter == GLOBAL ||
specificFilter.checkInput(serialClass) != Status.REJECTED;
case REJECTED -> false;
case UNDECIDED -> specificFilter.checkInput(serialClass) ==
Status.ALLOWED;
};
}
but I believe your proposed code reads better.
-------------
PR: https://git.openjdk.org/jdk/pull/10578
