fix(bedrock_agent): fix querystring field resolution #6777
Conversation
This fixes issue aws-powertools#6520 where Bedrock Agent parameters containing commas (like SQL queries) were being truncated because the resolved_query_string_parameters method was splitting them by commas. The fix overrides the resolved_query_string_parameters method in the BedrockAgentEvent class to preserve parameter values without splitting them by commas.
pull-request-size
bot
added
the
size/M
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #6777 +/- ##
===========================================
- Coverage 96.15% 96.13% -0.02%
===========================================
Files 256 256
Lines 12350 12354 +4
Branches 916 916
===========================================
+ Hits 11875 11877 +2
- Misses 373 375 +2
Partials 102 102 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Hi @matteofigus, thanks for submitting this PR! This solves the problem for BedrockResolver, but in other resolvers where we have multiquerystring fields, we have the same problem. For example, if you replace Bedrock Resolver with APIGatewayHttpResolver (of course, changing the payload), you will observe the same problem.
So, with that said, do you want to open a new issue to investigate other resolvers or do you want to address the issue in this same PR? I would prefer the second option, but feel free to make your own decision.
| @@ -112,6 +112,19 @@ def query_string_parameters(self) -> dict[str, str]: | |||
| parameters = self.get("parameters") or [] | |||
| return {x["name"]: x["value"] for x in parameters} | |||
|
|
|||
| @cached_property | |||
There was a problem hiding this comment.
I know we are using cached_property in the common base class, but tbh it doesn't have any effect here. We just access the headers once per request and when a new request arrives we create a new event source data class object, so, I'd prefer to make it @property to not save anything in cache.
Pls remove in the base class too https://github.com/aws-powertools/powertools-lambda-python/blob/develop/aws_lambda_powertools/utilities/data_classes/common.py#L172
| nonlocal received_query | ||
| received_query = query | ||
| return {"result": "Query executed"} |
There was a problem hiding this comment.
Just return query parameter and assert it. You don't need to have global/local variables here, it make the test unclear.
| result = app(event, {}) | ||
|
|
||
| # THEN the parameter with commas should be correctly passed to the handler | ||
| assert received_query == "SELECT a.source_name, b.thing FROM table" |
There was a problem hiding this comment.
After changing the return of the function, assert body["result"] here.
| @@ -62,3 +62,35 @@ def test_bedrock_agent_event_with_post(): | |||
| assert properties[1].name == raw_properties[1]["name"] | |||
| assert properties[1].type == raw_properties[1]["type"] | |||
| assert properties[1].value == raw_properties[1]["value"] | |||
|
|
|||
|
|
|||
| def test_bedrock_agent_event_with_comma_parameters(): | |||
There was a problem hiding this comment.
You can remove this test.
- Change @cached_property to @Property for resolved_query_string_parameters - Simplify the functional test by directly returning the query parameter - Remove redundant unit test
|
leandrodamascena
changed the title
Issue number: #6520
Summary
Changes
This PR fixes an issue where Bedrock Agent parameters containing commas (like SQL queries) were being truncated because the method was splitting them by commas.
The fix overrides the method in the class to preserve parameter values without splitting them by commas. This ensures that parameters like SQL queries with commas are correctly passed to the handler function.
Added unit and functional tests to verify the fix works correctly.
User experience
Before:
When using a Bedrock Agent with parameters containing commas (like SQL queries), only the part before the first comma was being passed to the handler function.
For example, with a parameter value of "SELECT a.source_name, b.thing FROM table", only "SELECT a.source_name" would be passed to the handler.
After:
The entire parameter value is correctly passed to the handler function, including any commas.
For example, with a parameter value of "SELECT a.source_name, b.thing FROM table", the entire string is passed to the handler.
Checklist
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.