Repository: apache/kafka
File: clients/src/main/java/org/apache/kafka/clients/consumer/internals/AutoOffsetResetStrategy.java (Java)

Code before:
        } else if (Arrays.asList(Utils.enumOptions(StrategyType.class)).contains(offsetStrategy)) {
            return true;
        } else if (offsetStrategy.startsWith(StrategyType.BY_DURATION + ":")) {
            String isoDuration = offsetStrategy.substring(StrategyType.BY_DURATION.toString().length() + 1);
            try {
                Duration duration = Duration.parse(isoDuration);
                return !duration.isNegative();
            } catch (Exception e) {
                return false;
            }
        } else {
            return false;
        }
    }

    /**
     *  Returns the AutoOffsetResetStrategy from the given string.
     */
    public static AutoOffsetResetStrategy fromString(String offsetStrategy) {
        if (!isValid(offsetStrategy)) {
            throw new IllegalArgumentException("Unknown auto offset reset strategy: " + offsetStrategy);
        }

        if (offsetStrategy.startsWith(StrategyType.BY_DURATION + ":")) {
            String isoDuration = offsetStrategy.substring(StrategyType.BY_DURATION.toString().length() + 1);
            return new AutoOffsetResetStrategy(StrategyType.BY_DURATION, Optional.of(Duration.parse(isoDuration)));
        }

        try {
            StrategyType type = StrategyType.valueOf(offsetStrategy.toUpperCase(Locale.ROOT));
            switch (type) {
                case EARLIEST:
                    return EARLIEST;
                case LATEST:
                    return LATEST;
                case NONE:
                    return NONE;
                default:
                    throw new IllegalArgumentException("Unknown auto offset reset strategy: " + offsetStrategy);
            }
        } catch (IllegalArgumentException e) {
            throw new IllegalArgumentException("Unknown auto offset reset strategy: " + offsetStrategy, e);
        }
    }

    /**
     * Returns the name of the offset reset strategy.
     */
    public String name() {
        return type.toString();
    }

Review comment (refactor):
we already had to do this same str processing/parsing to check `isValid`. Would it make sense maybe to have the validation returning the `AutoOffsetResetStrategy` object or null? It would simplify this fromString and keep the logic in one place (we could keep the isValid returning boolean if it can build a non-null AutoOffsetResetStrategy, and reuse that same builder here to build fromString?)

Code after:

        if (StrategyType.BY_DURATION.toString().equals(offsetStrategy)) {
            throw new IllegalArgumentException("<:duration> part is missing in by_duration auto offset reset strategy.");
        }

        if (Arrays.asList(Utils.enumOptions(StrategyType.class)).contains(offsetStrategy)) {
            StrategyType type = StrategyType.valueOf(offsetStrategy.toUpperCase(Locale.ROOT));
            switch (type) {
                case EARLIEST:
                    return EARLIEST;
                case LATEST:
                    return LATEST;
                case NONE:
                    return NONE;
                default:
                    throw new IllegalArgumentException("Unknown auto offset reset strategy: " + offsetStrategy);
            }
        }

        if (offsetStrategy.startsWith(StrategyType.BY_DURATION + ":")) {
            String isoDuration = offsetStrategy.substring(StrategyType.BY_DURATION.toString().length() + 1);
            try {
                Duration duration = Duration.parse(isoDuration);
                if (duration.isNegative()) {
                    throw new IllegalArgumentException("Negative duration is not supported in by_duration offset reset strategy.");
                }
                return new AutoOffsetResetStrategy(duration);
            } catch (Exception e) {
                throw new IllegalArgumentException("Unable to parse duration string in by_duration offset reset strategy.", e);
            }
        }

        throw new IllegalArgumentException("Unknown auto offset reset strategy: " + offsetStrategy);
    }

    /**
     * Returns the offset reset strategy type.
     */
    public StrategyType type() {
        return type;
    }

    /**
     * Returns the name of the offset reset strategy.
     */
    public String name() {
        return type.toString();
    }

    /**
     * Return the timestamp to be used for the ListOffsetsRequest.