Skip to content

Commit 445995a

Browse files
committed
added new new detection logic for conflicting service name and add sdk suffix
added new new new detection logic (next/Next token) for conflicting service name and add sdk suffix added a patch for Chatbotclient paginator
1 parent e742bc6 commit 445995a

File tree

7 files changed

+90
-60
lines changed

7 files changed

+90
-60
lines changed

generated/src/aws-cpp-sdk-s3/include/aws/s3/S3ClientPagination.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,13 @@ namespace Aws {
1515
namespace S3 {
1616

1717
using ListBucketsPaginator =
18-
Aws::Utils::Pagination::PagePaginator<S3Client, Model::ListBucketsRequest, Pagination::ListBucketsPaginationTraits>;
19-
using ListDirectoryBucketsPaginator =
20-
Aws::Utils::Pagination::PagePaginator<S3Client, Model::ListDirectoryBucketsRequest, Pagination::ListDirectoryBucketsPaginationTraits>;
21-
using ListObjectsV2Paginator =
22-
Aws::Utils::Pagination::PagePaginator<S3Client, Model::ListObjectsV2Request, Pagination::ListObjectsV2PaginationTraits>;
23-
using ListPartsPaginator = Aws::Utils::Pagination::PagePaginator<S3Client, Model::ListPartsRequest, Pagination::ListPartsPaginationTraits>;
18+
Aws::Utils::Pagination::PagePaginator<Aws::S3::S3Client, Aws::S3::Model::ListBucketsRequest, Pagination::ListBucketsPaginationTraits>;
19+
using ListDirectoryBucketsPaginator = Aws::Utils::Pagination::PagePaginator<Aws::S3::S3Client, Aws::S3::Model::ListDirectoryBucketsRequest,
20+
Pagination::ListDirectoryBucketsPaginationTraits>;
21+
using ListObjectsV2Paginator = Aws::Utils::Pagination::PagePaginator<Aws::S3::S3Client, Aws::S3::Model::ListObjectsV2Request,
22+
Pagination::ListObjectsV2PaginationTraits>;
23+
using ListPartsPaginator =
24+
Aws::Utils::Pagination::PagePaginator<Aws::S3::S3Client, Aws::S3::Model::ListPartsRequest, Pagination::ListPartsPaginationTraits>;
2425

2526
} // namespace S3
2627
} // namespace Aws

generated/src/aws-cpp-sdk-s3/include/aws/s3/model/pagination/ListBucketsPaginationTraits.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ namespace S3 {
1414
namespace Pagination {
1515

1616
struct ListBucketsPaginationTraits {
17-
using RequestType = Model::ListBucketsRequest;
18-
using ResultType = Model::ListBucketsResult;
19-
using OutcomeType = Model::ListBucketsOutcome;
20-
using ClientType = S3Client;
17+
using RequestType = Aws::S3::Model::ListBucketsRequest;
18+
using ResultType = Aws::S3::Model::ListBucketsResult;
19+
using OutcomeType = Aws::S3::Model::ListBucketsOutcome;
20+
using ClientType = Aws::S3::S3Client;
2121

2222
static OutcomeType Invoke(ClientType& client, const RequestType& request) { return client.ListBuckets(request); }
2323

generated/src/aws-cpp-sdk-s3/include/aws/s3/model/pagination/ListDirectoryBucketsPaginationTraits.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ namespace S3 {
1414
namespace Pagination {
1515

1616
struct ListDirectoryBucketsPaginationTraits {
17-
using RequestType = Model::ListDirectoryBucketsRequest;
18-
using ResultType = Model::ListDirectoryBucketsResult;
19-
using OutcomeType = Model::ListDirectoryBucketsOutcome;
20-
using ClientType = S3Client;
17+
using RequestType = Aws::S3::Model::ListDirectoryBucketsRequest;
18+
using ResultType = Aws::S3::Model::ListDirectoryBucketsResult;
19+
using OutcomeType = Aws::S3::Model::ListDirectoryBucketsOutcome;
20+
using ClientType = Aws::S3::S3Client;
2121

2222
static OutcomeType Invoke(ClientType& client, const RequestType& request) { return client.ListDirectoryBuckets(request); }
2323

generated/src/aws-cpp-sdk-s3/include/aws/s3/model/pagination/ListObjectsV2PaginationTraits.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ namespace S3 {
1414
namespace Pagination {
1515

1616
struct ListObjectsV2PaginationTraits {
17-
using RequestType = Model::ListObjectsV2Request;
18-
using ResultType = Model::ListObjectsV2Result;
19-
using OutcomeType = Model::ListObjectsV2Outcome;
20-
using ClientType = S3Client;
17+
using RequestType = Aws::S3::Model::ListObjectsV2Request;
18+
using ResultType = Aws::S3::Model::ListObjectsV2Result;
19+
using OutcomeType = Aws::S3::Model::ListObjectsV2Outcome;
20+
using ClientType = Aws::S3::S3Client;
2121

2222
static OutcomeType Invoke(ClientType& client, const RequestType& request) { return client.ListObjectsV2(request); }
2323

generated/src/aws-cpp-sdk-s3/include/aws/s3/model/pagination/ListPartsPaginationTraits.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ namespace S3 {
1414
namespace Pagination {
1515

1616
struct ListPartsPaginationTraits {
17-
using RequestType = Model::ListPartsRequest;
18-
using ResultType = Model::ListPartsResult;
19-
using OutcomeType = Model::ListPartsOutcome;
20-
using ClientType = S3Client;
17+
using RequestType = Aws::S3::Model::ListPartsRequest;
18+
using ResultType = Aws::S3::Model::ListPartsResult;
19+
using OutcomeType = Aws::S3::Model::ListPartsOutcome;
20+
using ClientType = Aws::S3::S3Client;
2121

2222
static OutcomeType Invoke(ClientType& client, const RequestType& request) { return client.ListParts(request); }
2323

tools/code-generation/smithy/codegen/cpp-pagination-codegen/src/main/java/com/amazonaws/util/awsclientsmithygenerator/generators/templates/PaginationClientHeaderGenerator.java

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,10 @@ public void render(CppWriter writer) {
4141
}
4242

4343
private void renderIncludes(CppWriter writer, String serviceName, String c2jServiceName) {
44-
writer.writeInclude("aws/" + c2jServiceName + "/" + serviceName + "Client.h");
44+
// Capitalize first letter to match C++ client naming convention
45+
String capitalizedServiceName = serviceName.substring(0, 1).toUpperCase() + serviceName.substring(1);
46+
47+
writer.writeInclude("aws/" + c2jServiceName + "/" + capitalizedServiceName + "Client.h");
4548
writer.writeInclude("aws/core/utils/pagination/Paginator.h");
4649

4750
for (OperationData<PaginatedTrait> data : paginatedOps) {
@@ -52,18 +55,21 @@ private void renderIncludes(CppWriter writer, String serviceName, String c2jServ
5255
}
5356

5457
private void renderNamespaces(CppWriter writer, String serviceName) {
58+
// Capitalize first letter to match C++ client naming convention
59+
String capitalizedServiceName = serviceName.substring(0, 1).toUpperCase() + serviceName.substring(1);
60+
5561
writer.writeNamespaceOpen("Aws");
56-
writer.writeNamespaceOpen(serviceName);
62+
writer.writeNamespaceOpen(capitalizedServiceName);
5763
writer.write("");
5864

5965
for (OperationData<PaginatedTrait> data : paginatedOps) {
6066
String opName = data.getOperation().getId().getName();
61-
writer.write("using $LPaginator = Aws::Utils::Pagination::PagePaginator<$LClient, Model::$LRequest, Pagination::$LPaginationTraits>;",
62-
opName, serviceName, opName, opName);
67+
writer.write("using $LPaginator = Aws::Utils::Pagination::PagePaginator<Aws::$L::$LClient, Aws::$L::Model::$LRequest, Pagination::$LPaginationTraits>;",
68+
opName, serviceName, capitalizedServiceName, serviceName, opName, opName);
6369
}
6470

6571
writer.write("");
66-
writer.writeNamespaceClose(serviceName);
72+
writer.writeNamespaceClose(capitalizedServiceName);
6773
writer.writeNamespaceClose("Aws");
6874
}
6975
}

tools/code-generation/smithy/codegen/cpp-pagination-codegen/src/main/java/com/amazonaws/util/awsclientsmithygenerator/generators/templates/PaginationTraitsGenerator.java

Lines changed: 56 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ public PaginationTraitsGenerator(PluginContext context, ServiceShape service, Li
3434
public void write() {
3535
String serviceName = ServiceNameUtil.getServiceName(service);
3636

37+
// TODO: Check why chatbotClientPagination.h is not generated as ChatbotClientPagination.h
38+
// The file name should follow the same capitalization pattern as other client files
39+
3740
for (OperationData<PaginatedTrait> data : paginatedOps) {
3841
String fileName = "include/aws/" + c2jServiceName + "/model/pagination/" + data.getOperation().getId().getName() + "PaginationTraits.h";
3942

@@ -50,6 +53,9 @@ private void generateTraitsHeader(CppWriter writer, OperationData<PaginatedTrait
5053
PaginatedTrait trait = data.getTrait();
5154
String opName = op.getId().getName();
5255

56+
// Capitalize first letter to match C++ client naming convention
57+
String capitalizedServiceName = serviceName.substring(0, 1).toUpperCase() + serviceName.substring(1);
58+
5359
// Header comment
5460
writer.write("/**")
5561
.write(" * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.")
@@ -60,25 +66,25 @@ private void generateTraitsHeader(CppWriter writer, OperationData<PaginatedTrait
6066

6167
// Includes - detect suffix like C2J renameShape logic
6268
String resultSuffix = getResultSuffix(opName);
63-
writer.writeInclude("aws/" + c2jServiceName + "/" + serviceName + "_EXPORTS.h")
69+
writer.writeInclude("aws/" + c2jServiceName + "/" + capitalizedServiceName + "_EXPORTS.h")
6470
.writeInclude("aws/" + c2jServiceName + "/model/" + opName + "Request.h")
6571
.writeInclude("aws/" + c2jServiceName + "/model/" + opName + resultSuffix + ".h")
66-
.writeInclude("aws/" + c2jServiceName + "/" + serviceName + "Client.h")
72+
.writeInclude("aws/" + c2jServiceName + "/" + capitalizedServiceName + "Client.h")
6773
.write("");
6874

6975
// Namespaces
7076
writer.writeNamespaceOpen("Aws")
71-
.writeNamespaceOpen(serviceName)
77+
.writeNamespaceOpen(capitalizedServiceName)
7278
.writeNamespaceOpen("Pagination")
7379
.write("");
7480

7581
// Struct definition
7682
writer.openBlock("struct " + opName + "PaginationTraits\n{", "};", () -> {
7783
// Use detected suffix to match C2J renameShape logic
78-
writer.write(" using RequestType = Model::$LRequest;", opName)
79-
.write(" using ResultType = Model::$L$L;", opName, resultSuffix)
80-
.write(" using OutcomeType = Model::$LOutcome;", opName)
81-
.write(" using ClientType = $LClient;", serviceName)
84+
writer.write(" using RequestType = Aws::$L::Model::$LRequest;", serviceName, opName)
85+
.write(" using ResultType = Aws::$L::Model::$L$L;", serviceName, opName, resultSuffix)
86+
.write(" using OutcomeType = Aws::$L::Model::$LOutcome;", serviceName, opName)
87+
.write(" using ClientType = Aws::$L::$LClient;", serviceName, capitalizedServiceName)
8288
.write("");
8389

8490
// Invoke method
@@ -153,12 +159,12 @@ private void generateTraitsHeader(CppWriter writer, OperationData<PaginatedTrait
153159

154160
writer.write("")
155161
.writeNamespaceClose("Pagination")
156-
.writeNamespaceClose(serviceName)
162+
.writeNamespaceClose(capitalizedServiceName)
157163
.writeNamespaceClose("Aws");
158164
}
159165

160166
// Replicate C2J renameShape conflict detection logic
161-
// TODO: This conflict detection logic may need to be moved to a shared utility class
167+
// TODO: This conflict detection logic may need to be moved to a shared utility class, its also very complicated and can be refactor
162168
// to avoid duplication between C2J and Smithy generators and ensure consistency.
163169
// Consider reusing the already implemented ShapeUtil class for shape name operations.
164170
private String getResultSuffix(String opName) {
@@ -167,42 +173,59 @@ private String getResultSuffix(String opName) {
167173
return "Response";
168174
}
169175

170-
// Replicate C2jModelToGeneratorModelTransformer.renameShape logic
171-
// Try suffixes in order: "Result", "SdkResult", "CppSdkResult"
172-
List<String> suffixOptions = Arrays.asList("Result", "SdkResult", "CppSdkResult");
176+
// For now, use the simple approach that works:
177+
// If the actual generated file exists, use Result; otherwise use SdkResult
173178

174-
for (String suffix : suffixOptions) {
175-
String candidateName = opName + suffix;
179+
// Check for known SdkResult cases (where data model conflicts exist)
180+
Set<Shape> allShapes = context.getModel().toSet();
181+
boolean hasDataModelConflict = allShapes.stream()
182+
.anyMatch(shape -> {
183+
String shapeName = shape.getId().getName();
184+
if (shapeName.equals(opName + "Result")) {
185+
// Found a shape with the same name - check if it's a data model
186+
if (shape instanceof StructureShape) {
187+
StructureShape structShape = (StructureShape) shape;
188+
// If it doesn't have NextToken/nextToken, it's likely a data model
189+
Set<String> memberNames = structShape.getAllMembers().keySet();
190+
// TODO: Sanitize member names for other edge cases (e.g., different casing, underscores, etc.)
191+
boolean hasNextToken = memberNames.contains("NextToken") || memberNames.contains("nextToken");
192+
return !hasNextToken;
193+
}
194+
}
195+
return false;
196+
});
176197

177-
// Check if there would be a naming conflict with this suffix
178-
if (!hasNamingConflict(candidateName, opName)) {
179-
return suffix;
180-
}
181-
}
182-
183-
// Fallback to "Result" if no conflicts detected
184-
return "Result";
198+
return hasDataModelConflict ? "SdkResult" : "Result";
185199
}
186200

187-
// Replicate the conflict detection logic from C2jModelToGeneratorModelTransformer
201+
// TODO: Delete this method if it's not used - replaced by simpler conflict detection in getResultSuffix
202+
// Replicate the precise conflict detection logic from C2jModelToGeneratorModelTransformer
188203
private boolean hasNamingConflict(String candidateName, String opName) {
189-
// Check if there's a shape that would conflict with this name
190-
// This mimics the conflict detection in renameShape method lines 771-775
191-
192204
// Get all shapes in the model to check for conflicts
193205
Set<Shape> allShapes = context.getModel().toSet();
194206

195207
for (Shape shape : allShapes) {
196208
String shapeName = shape.getId().getName();
197209

198-
// Check if the candidate name conflicts with existing shape names
210+
// Direct exact name conflict - this is the main case
199211
if (candidateName.equals(shapeName)) {
200-
// There's a direct name conflict, need to use next suffix
201-
return true;
202-
}
203-
204-
// Check for "Get" + shapeName pattern conflicts (from C2J logic)
205-
if (candidateName.equals("Get" + shapeName) || candidateName.equals("Set" + shapeName)) {
212+
// If this is a structure, check if it's already a suitable operation result
213+
if (shape instanceof StructureShape) {
214+
StructureShape structShape = (StructureShape) shape;
215+
// If the existing shape has pagination tokens, it's already an operation result - no conflict
216+
// Check for various pagination token field names
217+
boolean hasNextToken = structShape.getAllMembers().keySet().stream()
218+
.anyMatch(memberName -> memberName.toLowerCase().contains("nexttoken") ||
219+
memberName.toLowerCase().contains("token"));
220+
221+
if (hasNextToken) {
222+
// This is already an operation result shape, no conflict
223+
return false;
224+
}
225+
226+
// If it doesn't have pagination tokens, it's a data model - conflict!
227+
return true;
228+
}
206229
return true;
207230
}
208231
}

0 commit comments

Comments
 (0)