Fix protocol initialization for AwsRestJson1Protocol

[yasmewad]

Issue #, if available:

Fixes #1017

Description of changes:

Root Cause:
When AwsRestJson1Protocol (or other HTTP-based protocols) are present in the classpath, the ProtocolResolver instantiates them with all available services. The AwsRestJson1 protocol constructor iterates through all operations and calls expectTrait(HTTP_TRAIT), which throws when operations don't have the HTTP trait. CBOR services won't have that trait.

Changes

• Changed the protocol handler for AwsRestJson1 to correctly use getTrait instead of expectTrait which throws exception, and let it pass through.
• Renamed the word priority to precision to be consistent with description in Wire protocol selection docs.
• Added unit tests for the protocol handlers.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[adwsingh]

With the RestJson protocol, all operations are required to have the HTTP trait, so this situation should not arise in the first place.

On that note, I don’t think this addresses the root cause of the problem, it only treats a symptom.

In my opinion, the correct fix is to make the protocol resolver filter and initialize only the providers that are actually required, based on the protocol traits declared on the service.

[yasmewad]

That's a fair callout. I looked into protocol-level filtering first but hit some blockers.

ProtocolResolver gets instantiated with the Server and supports multiple services simultaneously. Consider serviceA with restJson1 and rpcv2Cbor, and serviceB with only rpcv2Cbor. We'd still need handlers for both protocols loaded. Filtering at the constructor level wouldn't work for this case.

I tried implementing filtering anyway and ran into the TraitKey issue. ProtocolTestProtocolProvider provides a ShapeId but no trait class to construct a TraitKey from. Also not sure if protocol traits are even available in the schema at that point since some example services don't define protocols explicitly.

To unblock the issue and open up further discussion, and going through the options with Manuel, I decided to make HTTP_TRAIT retrieval defensive. This lets handlers instantiate even when services don't use restJson1 or have HTTP traits.

My understanding is that the server needs to be aware of all protocol handlers on the classpath to select the appropriate one at runtime. I think changing this pattern could be challenging, though I might be missing something.

In my opinion, the cleanest solution would be adding a filterCandidateServices method to ServerProtocolProvider. Each provider could implement its own filtering logic based on the wire protocol selection spec instead of failing during construction.

What do you think about that approach?

[yasmewad]

Merging this and will continue the discussion offline.