Removed unnessary checks in S3 sink

[Claudenw]

cleaned up code and added javadocs.

From a Slack conversation.

Q: I was looking at S3SinkConfig and came across this.

 @Override
    public List<OutputField> getOutputFields() {
        if (Objects.nonNull(getList(FORMAT_OUTPUT_FIELDS_CONFIG))
                && !get(FORMAT_OUTPUT_FIELDS_CONFIG).equals(ConfigDef.NO_DEFAULT_VALUE)) {
            return getOutputFields(FORMAT_OUTPUT_FIELDS_CONFIG);
        }
        if (Objects.nonNull(getList(OUTPUT_FIELDS)) && !get(OUTPUT_FIELDS).equals(ConfigDef.NO_DEFAULT_VALUE)) {
            return getOutputFields(OUTPUT_FIELDS);
        }
        return List.of(new OutputField(OutputFieldType.VALUE, OutputFieldEncodingType.BASE64));
    }

Now FORMAT_OUTPUT_FIELDS_CONFIG is defined as a List with the default of null.

so checking if Objects.nonNull(getList(FORMAT_OUTPUT_FIELDS_CONFIG) makes sense.

But this part I don't get && !get(FORMAT_OUTPUT_FIELDS_CONFIG).equals(ConfigDef.NO_DEFAULT_VALUE)) If NO_DEFAULT_VALUE is specified as the default when the ConfigDef is created, then when the map of key/value pairs is passed in if there is no value in the map the NO_DEFAULT_VALUE causes a ConfigException to be thrown. So I don't see any way that the value of the property can be a NO_DEFAULT_VALUE , it is either null, or a list.

I missing something?

A: Yeah that stuff doesn't make sense. NO_DEFAULT_VALUE is a sentinel for the validation logic, it's not a real default.

[aindriu-aiven]

LGTM

[RyanSkraba]

This formatting change is a bit irritating, because these // NOPMD comments shouldn't be split across lines in the first place. 🙄

Technically only the last one in this list is actually necessary anyway, and we're ignoring checks that wouldn't fail.

[RyanSkraba]

Suggested change

	* @return The list of output fields. WIll not be {@code null}.
	* @return The list of output fields. Will not be {@code null}.

[RyanSkraba]

LGTM even as it is -- those little nitpicks are extremely minor, do you want to fix them here?

[aindriu-aiven]

@RyanSkraba merged as these were nits and I want to get the number of open PRs down.