Bug fixes and refactoring

[msfroh]

1. Shallow clone SearchResponse in SearchActionFilter. The only properties that we're currently changing are SearchHit and timeTookMillis. This is also less brittle as we don't try to implement deserialization ourselves.
2. KendraIntelligentRanker doesn't transform if the required client settings (endpoint + execution plan ID) are missing.
3. A lot of refactoring to prepare for other transformers. Where we previously had logic of the form if kendra_intelligent_ranking then do Kendra intelligent ranking, now the APIs support arbitrary named transformers. This required updates to tests and configuration code.
4. Added tests and fixes for SearchConfigurationExtBuilder.

Signed-off-by: Michael Froh froh@amazon.com

Description

See commit message above.

Issues Resolved

• Refactoring is a prerequisite for [RFC] Additional Reranker Implementations Requested #36
• Additional tests contribute to Improve test coverage #31

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Commits are signed as per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[noCharger]

I guess setting class is more specific towards that transformer itself, whereas the configuration is the schema of the search workflow. There are two things I am not sure if we should put in this PR:

1. Can I use multiple same reranker, with different setting and configuration?
2. Do we want to provide more flexibility, such as one transformer depend on several other transformers.

[codecov-commenter]

Codecov Report

Merging #38 (698f275) into main (d9b44e7) will increase coverage by 5.09%.
The diff coverage is 59.60%.

@@             Coverage Diff              @@
##               main      #38      +/-   ##
============================================
+ Coverage     60.43%   65.52%   +5.09%     
- Complexity      155      172      +17     
============================================
  Files            29       29              
  Lines           867      879      +12     
  Branches        119      119              
============================================
+ Hits            524      576      +52     
+ Misses          304      254      -50     
- Partials         39       49      +10     

Impacted Files	Coverage Δ
...search/search/relevance/SearchRelevancePlugin.java	0.00% <0.00%> (ø)
.../configuration/ResultTransformerConfiguration.java	100.00% <ø> (ø)
...configuration/KendraIntelligentRankerSettings.java	76.66% <ø> (ø)
.../KendraIntelligentRankingConfigurationFactory.java	0.00% <0.00%> (ø)
...draintelligentranking/client/KendraHttpClient.java	21.56% <23.33%> (+21.56%)	⬆️
...uration/KendraIntelligentRankingConfiguration.java	26.53% <50.00%> (-7.82%)	⬇️
...ch/relevance/configuration/ConfigurationUtils.java	66.66% <60.00%> (ø)
...e/configuration/SearchConfigurationExtBuilder.java	67.64% <66.66%> (+53.61%)	⬆️
...draintelligentranking/KendraIntelligentRanker.java	67.61% <91.66%> (+0.29%)	⬆️
...rch/relevance/actionfilter/SearchActionFilter.java	84.76% <96.00%> (+3.72%)	⬆️
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[noCharger]

Non-blocking:

This is what I preferred:

    if (!isValid()) {
      amazonHttpClient = null;
      aws4Signer = null;
      awsCredentialsProvider = null;
      errorHandler = null;
      responseHandler = null;
      return;
    }
    amazonHttpClient = AccessController.doPrivileged((PrivilegedAction<AmazonHttpClient>) () -> new AmazonHttpClient(new ClientConfiguration()));
    errorHandler = new SimpleAwsErrorHandler();
    responseHandler = new SimpleResponseHandler();
    aws4Signer = new AWS4Signer();
    aws4Signer.setServiceName(KENDRA_RANKING_SERVICE_NAME);
    aws4Signer.setRegionName(clientSettings.getServiceRegion());
   ...

[noCharger]

This is awesome! Approved with non-blocking comments.

[macohen]

We do want to review the full set of metrics and logging events to capture. We'll create another issue for that purpose.