On Mon, 10 Oct 2022 13:14:34 GMT, Daniel Fuchs <[email protected]> wrote:
>>> 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 -> filter != GLOBAL &&
> specificFilter.checkInput(serialClass) == Status.ALLOWED;
> };
> }
>
>
> but I believe your proposed code reads better.
Thanks - pushed as 528489b
-------------
PR: https://git.openjdk.org/jdk/pull/10578