Pure refactoring of UpdateItemOperation tests #2910
Conversation
| PRIMARY_CONTEXT, | ||
| null); | ||
| UpdateItemRequest.Builder expectedRequestBuilder = ddbRequestBuilder(ddbKey(item.getId(), item.getSort())); | ||
| expectedRequestBuilder.expressionAttributeValues(expectedValues); |
There was a problem hiding this comment.
Nit: I think it helps readability to chain the builder method calls together, rather than repeatedly reference expectedRequestBuilder. Same feedback for other locations.
There was a problem hiding this comment.
Agreed... I stopped refactoring these types of tests after a while because I'll be adding quite a few similar ones, and I didn't want to end up having to back out of some optimization. I would like to simplify the handling of the updateExpression/expressionNames/expressionMaps/conditionals to a point where it's easy to glance at a test and quickly see the significant bits. Expect to see something in the next PR.
| UpdateItemRequest request = updateItemOperation.generateRequest(FakeItemWithSort.getTableSchema(), PRIMARY_CONTEXT, null); | ||
|
|
||
|
|
||
| String expectedUpdateExpression = "SET " + OTHER_ATTRIBUTE_1_NAME + " = " + OTHER_ATTRIBUTE_1_VALUE + |
There was a problem hiding this comment.
Nit: As soon as we exceed a single concatenation operator, I usually find String.format(..) to be more readable.
There was a problem hiding this comment.
Yeah, didn't love this setup either but again, because we're releasing support of the full range of UpdateExpression, there will be a lot of different types of expression strings so it'll be rectified when those are added.
|
Kudos, SonarCloud Quality Gate passed! |
Revert "Support Non proxy host settings in the ProxyConfiguration for Crt http client. (#4962)"
It's a big class and we'll need to add more tests.