KAFKA-17563: Move RequestConvertToJson to server module

[xijiu]

As title.

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[chia7712]

@xijiu thanks for this patch.

Could you please move some tests from RequestConvertToJsonTest to server module? testClientInfoNode, testAllRequestTypesHandled, testAllApiVersionsResponseHandled and testAllResponseTypesHandled

[xijiu]

@xijiu thanks for this patch.

Could you please move some tests from RequestConvertToJsonTest to server module? testClientInfoNode, testAllRequestTypesHandled, testAllApiVersionsResponseHandled and testAllResponseTypesHandled

@chia7712 Sure, I have migrated these four methods, PTAL

[mimaison]

I wonder if we should wait for server to switch to Java 17 before merging this. With Java 17, we will be able to use pattern matching and significantly simplify the logic. The huge list of if (x instance y) is really ugly with Java 11.

[chia7712]

I wonder if we should wait for server to switch to Java 17 before merging this. With Java 17, we will be able to use pattern matching and significantly simplify the logic. The huge list of if (x instance y) is really ugly with Java 11.

That is a good idea. Not only match pattern but also record type can simplify Kafka code base significantly. Also, I agree to that instanceof is ugly. Maybe we can use ApiKeys instead? for exampel:

        switch(request.apiKey()) {
            case PRODUCE:
                return ProduceRequestDataJsonConverter.write(((ProduceRequest) request).data(), request.version(), false);
        }

That is more graceful and probably have better performance.

[mimaison]

Maybe we can use ApiKeys instead?

Yes that's also an option. I'd really prefer this or pattern matching over the current code.

[xijiu]

Maybe we can use ApiKeys instead?

Yes that's also an option. I'd really prefer this or pattern matching over the current code.

@mimaison @chia7712 Thanks very much for code review, and indeed, the switch(request.apiKey()) mode is more elegant and efficient

I have changed it, PTAL