Skip to content

Add kiro steering docs #6280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 133 additions & 0 deletions .kiro/steering/async-programming-guidelines.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
---
title: Asynchronous Programming Guidelines for AWS SDK v2
inclusion: fileMatch
fileMatchPattern: "**/*.java"
---

# Asynchronous Programming Guidelines for AWS SDK v2

## Use of CompletableFuture

### Best Practices for CompletableFuture

- **Read the documentation**: Always read the [CompletionStage Javadocs](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletionStage.html) to understand the nuances of CompletableFuture
- **Prefer Non-Blocking Methods for Getting Results**:

```java
// Avoid when possible - blocks the current thread
String result = future.get();

// Better - use callbacks
future.thenAccept(result -> processResult(result));
```
- **Add stacktrace to exceptions**: When using `CompletableFuture#join`, use `CompletableFutureUtils#joinLikeSync` to preserve stacktraces
- **Don't ignore results**: Never ignore the result of a new `CompletionStage` if the parent stage can fail (unless you're absolutely sure it's safe to)
- **Don't make thread assumptions**: Never make assumptions about which thread completes the future
- CompletableFuture callbacks may execute on:
- The thread that completed the future
- A thread from the common ForkJoinPool (for async methods without explicit executor)
- A thread from a provided executor
- Thread behavior can vary based on:
- Whether the future is already completed when a callback is added
- Whether an async or non-async method is used (e.g., `thenApply` vs `thenApplyAsync`)
- The specific JVM implementation and platform
- Always ensure thread safety when:
- Accessing shared state in callbacks
- Modifying UI components (use the appropriate UI thread)
- Working with thread-affinity resources like ThreadLocals
- Example of incorrect assumption:
```java
// INCORRECT: Assuming the callback runs on a specific thread
ThreadLocal<Context> contextHolder = new ThreadLocal<>();

public void processAsync(CompletableFuture<Response> responseFuture) {
Context context = new Context();
contextHolder.set(context); // Set in current thread

responseFuture.thenApply(response -> {
// WRONG: Assuming contextHolder still has the context
Context ctx = contextHolder.get(); // May be null if running on different thread!
return processWithContext(response, ctx);
});
}
```
- Correct approach:
```java
// CORRECT: Explicitly passing context to callback
public void processAsync(CompletableFuture<Response> responseFuture) {
Context context = new Context();

responseFuture.thenApply(response -> {
// Explicitly use the context passed from the outer scope
return processWithContext(response, context);
});
}
```
- **Always provide custom executors**: Don't use `CompletableFuture#xxAsync` methods (like `runAsync` or `thenComposeAsync`) without providing a custom executor, as the default `ForkJoinPool.commonPool()` behavior can vary by platform
- **Handle cancellation properly**: CompletableFuture does not support automatic cancellation propagation, so use `CompletableFutureUtils#forwardExceptionTo` to manually propagate cancellation
- **Avoid chaining multiple API calls**: This can lead to cancellation issues without proper handling
- **Avoid blocking operations**: Never use `get()` or `join()` inside a CompletableFuture chain as it defeats the purpose of asynchronous execution
- **Handle exceptions properly**: Always include exception handling with `exceptionally()` or `handle()` methods
```java
CompletableFuture.supplyAsync(() -> fetchData())
.exceptionally(ex -> {
logger.error("Error processing data", ex);
return fallbackData();
}, executor);
```
- **Use appropriate completion methods**:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about whenComplete? We use it a lot

- thenApply() - when transforming a result
- thenAccept() - when consuming a result without returning anything
- thenRun() - when executing code regardless of the result
- thenCompose() - when the next step returns a CompletableFuture
- **Test asynchronous code properly**:
- Use `CompletableFuture.join()` in tests to wait for completion
- Set appropriate timeouts for tests

## Reactive Streams Implementation

The AWS SDK for Java v2 uses Reactive Streams for asynchronous, non-blocking data processing with backpressure support. All implementations must adhere to the following requirements:

### Compliance Requirements

- All implementations **MUST** fully comply with the [Reactive Streams Specification](https://github.com/reactive-streams/reactive-streams-jvm)
- All implementations **MUST** pass the [Reactive Streams Technology Compatibility Kit (TCK)](https://github.com/reactive-streams/reactive-streams-jvm/tree/master/tck) tests
- Any code changes to Reactive Streams implementations **MUST** include TCK verification tests

### Implementation Guidelines

- Publishers **MUST** respect backpressure signals from subscribers
- Publishers **MUST** properly propagate cancellation signals
- Publishers **MUST** properly handle and propagate errors
- Subscribers **MUST** handle onNext, onError, and onComplete signals appropriately
- Subscribers **MUST** send appropriate request(n) signals to maintain backpressure
- Processors **MUST** maintain all Publisher and Subscriber contracts
- Developers **SHOULD NOT** implement new Publisher or Subscriber interfaces from scratch
- Developers **SHOULD** utilize existing utility classes such as:
- `SimplePublisher` - A simple implementation of the Publisher interface
- `ByteBufferStoringSubscriber` - A subscriber that stores received ByteBuffers
- Methods in `SdkPublisher` - Utility methods for common Publisher operations

### Common Patterns

- Use `SdkPublisher` for SDK-specific publisher implementations
- Implement proper resource cleanup in both success and failure scenarios
- Handle cancellation gracefully, including cleanup of any allocated resources
- Ensure thread safety in all implementations
- Document thread safety characteristics and any assumptions about execution context

### Testing Requirements

- All Reactive Streams implementations **MUST** include TCK verification tests
- Tests **SHOULD** cover both normal operation and edge cases:
- Cancellation during active streaming
- Error propagation
- Backpressure handling under various request scenarios
- Resource cleanup in all termination scenarios

## References

- [CompletableFuture JavaDoc](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html)
- [CompletionStage JavaDoc](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletionStage.html)
- [Reactive Streams Specification](https://github.com/reactive-streams/reactive-streams-jvm)
- [AWS SDK for Java Developer Guide - Async Programming](https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/asynchronous.html)
162 changes: 162 additions & 0 deletions .kiro/steering/aws-sdk-java-v2-general.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
---
title: AWS SDK for Java v2 General Guidelines
inclusion: always
---

# AWS SDK for Java v2 General Guidelines

## General Principles

- Write clean, readable, and maintainable code
- Follow the SOLID principles of object-oriented design
- Favor composition over inheritance
- Program to interfaces, not implementations
- Fail fast - detect and report errors as soon as possible
- Maintain backward compatibility when modifying existing APIs

## Code Style Standards

- Follow Java coding conventions and the existing style in the codebase
- Use meaningful variable, method, and class names that clearly indicate purpose
- Include comprehensive Javadoc for public APIs
- Keep methods short and focused on a single responsibility
- Limit the number of method parameters (ideally 3 or fewer)
- Use appropriate access modifiers (private, protected, public)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will Kiro know what is "appropriate" or should we elaborate?

- Follow consistent indentation and formatting

## Common Design Patterns

- Use builder pattern for object creation
- Follow reactive programming principles for async operations
- Use SdkException hierarchy for error handling
- Prefer immutable objects where appropriate

## Naming Conventions

### Class Naming

#### General Rules
- Prefer singular class names: `SdkSystemSetting`, not `SdkSystemSettings`
- Treat acronyms as a single word: `DynamoDbClient`, not `DynamoDBClient`

#### Classes that instantiate other classes

- If the class's primary purpose is to return instances of another class:
- If the "get" method has no parameters:
- If the class implements `Supplier`: `{Noun}Supplier` (e.g. `CachedSupplier`)
- If the class does not implement `Supplier`: `{Noun}Provider` (e.g. `AwsCredentialsProvider`)
- If the "get" method has parameters: `{Noun}Factory` (e.g. `AwsJsonProtocolFactory`)

#### Service-specific classes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this section seems a bit restrictive? We don't want any other options?


- If the class makes service calls:
- If the class can be used to invoke *every* data-plane operation:
- If the class is code generated:
- If the class uses sync HTTP: `{ServiceName}Client` (e.g. `DynamoDbClient`)
- If the class uses async HTTP: `{ServiceName}AsyncClient` (e.g. `DynamoDbAsyncClient`)
- If the class is hand-written:
- If the class uses sync HTTP: `{ServiceName}EnhancedClient` (e.g. `DynamoDbEnhancedClient`)
- If the class uses async HTTP: `{ServiceName}EnhancedAsyncClient` (e.g. `DynamoDbEnhancedAsyncClient`)
- If the class can be used to invoke only *some* data-plane operations:
- If the class uses sync HTTP: `{ServiceName}{Noun}Manager` (e.g. `SqsBatchManager`)
- If the class uses async HTTP: `{ServiceName}Async{Noun}Manager` (e.g. `SqsAsyncBatchManager`)
- Note: If only one implementation exists and it uses async HTTP, `Async` may be excluded. (e.g. `S3TransferManager`)
- If the class does not make service calls:
- If the class creates presigned URLs: `{ServiceName}Presigner` (e.g. `S3Presigner`)
- If the class is a collection of various unrelated "helper" methods: `{ServiceName}Utilities` (e.g. `S3Utilities`)

## Class Initialization

- Favor static factory methods over constructors:
- Static factory methods provide meaningful names compared with constructors
- They are useful when working with immutable classes as we can reuse the same object
- Static factory methods can return any subtype of that class

### Naming Conventions for Static Factory Methods
- `create()`, `create(params)` when creating a new instance (e.g., `DynamoDBClient.create()`)
- `defaultXXX()` when returning an instance with default settings (e.g., `BackoffStrategy.defaultStrategy()`)

## Use of Optional

- `Optional` **MUST NOT** be used when the result will never be null
- For return types:
- `Optional` **SHOULD** be used when it is not obvious to a caller whether a result will be null
- `Optional` **MUST NOT** be used for "getters" in generated service model classes
- For member variables: `Optional` **SHOULD NOT** be used
- For method parameters: `Optional` **MUST NOT** be used
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we provide guidance to use @nullable instead or just not doing it?


## Object Methods (toString, equals, hashCode)

- All public POJO classes **MUST** implement `toString()`, `equals()`, and `hashCode()` methods
- When adding new fields to existing POJO classes, these methods **MUST** be updated to include the new fields
- Implementation guidelines:
- `toString()`: Include class name and all fields with their values
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible additional guideline - do not include fields that could be considered sensitive such as credentials

- **MUST** use the SDK's `ToString` utility class (`utils/src/main/java/software/amazon/awssdk/utils/ToString.java`) for consistent formatting:
```java
@Override
public String toString() {
return ToString.builder("YourClassName")
.add("fieldName1", fieldValue1)
.add("fieldName2", fieldValue2)
.build();
}
```
- `equals()`: Compare all fields for equality, including proper null handling
- `hashCode()`: Include all fields in the hash code calculation
- Consider using IDE-generated implementations
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SDK ToString utils for toString()?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this guidance useful for kiro? I think we often use the intelliJ auto-generated implementations, but not sure that will apply well here. I think the guidance maybe should be either: use java.util.Objects helpers or write explicit code.

- For immutable objects, consider caching the hash code value for better performance
- Unit tests **MUST** be added using EqualsVerifier to ensure all fields are properly included in equals and hashCode implementations. Example:
```java
@Test
public void equalsHashCodeTest() {
EqualsVerifier.forClass(YourClass.class)
.withNonnullFields("requiredFields")
.verify();
}
```

## Exception Handling

- Avoid throwing checked exceptions in public APIs
- Don't catch exceptions unless you can handle them properly
- Always include meaningful error messages
- Clean up resources in finally blocks or use try-with-resources
- Don't use exceptions for flow control

## Use of Existing Utility Classes

- Developers **MUST** check for existing utility methods before implementing their own
- Common utility classes are available in the `utils` module and **SHOULD** be used when applicable
- Examples of available utility classes:
- `Lazy` (`utils/src/main/java/software/amazon/awssdk/utils/Lazy.java`): For thread-safe lazy initialization of singleton objects
- `ToString` (`utils/src/main/java/software/amazon/awssdk/utils/ToString.java`): For consistent toString() implementations
- `IoUtils`: For safely closing resources and handling I/O operations
- `StringUtils`: For common string operations
- `CollectionUtils`: For common collection operations
- `ValidationUtils`: For input validation
- Using existing utilities ensures:
- Consistent behavior across the codebase
- Thread-safety where applicable
- Proper handling of edge cases
- Reduced code duplication
- Example of using `Lazy` for thread-safe singleton initialization:
```java
private static final Lazy<ExpensiveObject> INSTANCE = new Lazy<>(() -> new ExpensiveObject());

public static ExpensiveObject getInstance() {
return INSTANCE.getValue();
}
```

## Performance

- Avoid premature optimization
- Use appropriate data structures for the task
- Be mindful of memory usage, especially with large collections
- Consider thread safety in concurrent applications
- Use profiling tools to identify actual bottlenecks

## References

- [AWS SDK for Java Developer Guide](https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/home.html)
- [Design Documentation](https://github.com/aws/aws-sdk-java-v2/tree/master/docs/design)
44 changes: 44 additions & 0 deletions .kiro/steering/aws-sdk-java-v2-guidelines.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
title: AWS SDK for Java v2 Development Guidelines
inclusion: always
---

# AWS SDK for Java v2 Development Guidelines

This document serves as an index to the AWS SDK for Java v2 development guidelines. The guidelines have been organized into separate, focused documents to make them more manageable and easier to reference.

## Available Guidelines

### [General Guidelines](aws-sdk-java-v2-general.md)
- General Principles
- Code Style Standards
- Common Design Patterns
- Naming Conventions
- Class Initialization
- Use of Optional
- Exception Handling
- Performance
- Documentation

### [Asynchronous Programming Guidelines](async-programming-guidelines.md)
- Use of CompletableFuture
- Reactive Streams Implementation

### [Testing Guidelines](testing-guidelines.md)
- General Testing Principles
- Test Naming Conventions
- Unit Testing Best Practices
- Testing Asynchronous Code

### [Javadoc Guidelines](javadoc-guidelines.md)
- API Classification
- Documentation Requirements
- Style Guidelines
- Code Snippets
- Examples

## References

- [AWS SDK for Java Developer Guide](https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/home.html)
- [Design Documentation](https://github.com/aws/aws-sdk-java-v2/tree/master/docs/design)
- [Migration Guide](https://docs.aws.amazon.com/sdk-for-java/v2/migration-guide/what-is-java-migration.html)
Loading
Loading