-
Notifications
You must be signed in to change notification settings - Fork 0
Added new example of reactive #1
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
Conversation
|
Warning Rate limit exceeded@developer-shubham101 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 13 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis change introduces a new Spring Boot reactive application scaffolded under the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant BlogController
participant BlogService
participant BlogRepository
participant MongoDB
Client->>BlogController: HTTP request (e.g., POST /blogs)
BlogController->>BlogService: createBlog(blog)
BlogService->>BlogRepository: save(blog)
BlogRepository->>MongoDB: Insert document
MongoDB-->>BlogRepository: Ack/Saved Blog
BlogRepository-->>BlogService: Mono<Blog>
BlogService-->>BlogController: Mono<Blog>
BlogController-->>Client: HTTP response (Mono<Blog>)
sequenceDiagram
participant Client
participant EpisodeController
participant WebClient
participant ExternalAPI
participant Tracer
Client->>EpisodeController: GET /episodes
EpisodeController->>Tracer: Start trace/span
EpisodeController->>WebClient: GET external API
WebClient->>ExternalAPI: HTTP GET request
ExternalAPI-->>WebClient: API response (episodes)
WebClient-->>EpisodeController: Flux<EpisodeResponse>
EpisodeController->>Tracer: End trace/span
EpisodeController-->>Client: HTTP response (List<CustomEpisode>)
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
🧹 Nitpick comments (18)
reactive/.mvn/wrapper/maven-wrapper.properties (1)
1-16: Consider adding a separating blank line
Introduce a blank line between the ASF license header and the property declarations to improve readability and maintain consistency with other configuration files.reactive/src/main/resources/application.yml (1)
5-7: Externalize MongoDB URI via environment variable
Hard-codingmongodb://localhost:27017/reactive-dbinapplication.ymlcan hinder flexibility across environments. Consider using a placeholder that reads from an environment variable, e.g.:spring: data: mongodb: uri: ${MONGODB_URI:mongodb://localhost:27017/reactive-db}reactive/src/main/java/com/example/reactive/sample/EpisodeResponse.java (1)
3-9: Add equals(), hashCode(), and toString() methodsFor completeness and to ensure proper behavior when using this class in collections or debugging, consider adding these standard methods.
You can use your IDE's code generation feature or Lombok's
@Dataannotation to generate these methods automatically.reactive/src/main/java/com/example/reactive/sample/WebClientConfig.java (1)
7-13: Add base URL configuration for the WebClientIf the WebClient is primarily used for a specific API (like the Futurama API mentioned in the summary), consider configuring a base URL.
@Configuration public class WebClientConfig { + @Value("${api.futurama.base-url:https://api.sampleapis.com/futurama}") + private String baseUrl; + @Bean public WebClient webClient(WebClient.Builder builder) { - return builder.build(); + return builder + .baseUrl(baseUrl) + .build(); } }reactive/src/main/java/com/example/reactive/BlogRepository.java (2)
6-8: Add documentation for better developer experienceThe repository interface is correctly implemented, but adding JavaDoc would improve documentation and maintainability.
+/** + * Repository for Blog entities, providing standard CRUD operations and custom queries. + */ public interface BlogRepository extends ReactiveMongoRepository<Blog, String> { + /** + * Find all blogs by a specific author. + * + * @param author The author's name to search for + * @return A flux of blogs written by the specified author + */ Flux<Blog> findByAuthor(String author); }
6-8: Consider adding additional query methods for common use casesDepending on your application requirements, you might want to add more query methods.
public interface BlogRepository extends ReactiveMongoRepository<Blog, String> { Flux<Blog> findByAuthor(String author); + + Flux<Blog> findByTitleContaining(String titleFragment); + + Mono<Blog> findFirstByAuthorOrderByCreatedAtDesc(String author); }reactive/src/main/java/com/example/reactive/sample/CustomEpisode.java (1)
3-16: Consider adding validation and missing utility methodsThis model class would benefit from:
- Field validation (e.g.,
@NotBlankfor required String fields)- Implementation of
equals(),hashCode(), andtoString()methods- A more descriptive name than "CustomEpisode"
Using Lombok annotations like
@Data(similar to theBlogclass) would automatically generate these utility methods and reduce boilerplate code.+import lombok.Data; +import javax.validation.constraints.NotBlank; +import javax.validation.constraints.NotNull; +@Data public class CustomEpisode { - public String title; - public String writers; - public String publishDate; - public String description; - public int id; + @NotBlank + private String title; + @NotBlank + private String writers; + private String publishDate; + private String description; + @NotNull + private int id;reactive/src/main/java/com/example/reactive/Blog.java (1)
8-16: Consider adding indexes for query performanceIf
authoror other fields will be frequently used in queries, consider adding MongoDB indexes to improve query performance.@Data @Document(collection = "blogs") +// To optimize queries on author field +@org.springframework.data.mongodb.core.index.CompoundIndex( + name = "author_idx", + def = "{'author': 1}" +) public class Blog { @Id private String id; private String title; private String content; private String author; private LocalDateTime createdAt = LocalDateTime.now(); }reactive/pom.xml (3)
16-28: Clean up or complete the empty project metadata elementsThe POM contains empty elements for
<url/>,<license/>,<developer/>, and<scm>related tags. Either complete these elements with appropriate information or remove them to keep the POM clean and maintainable.<version>0.0.1-SNAPSHOT</version> <name>reactive</name> <description>Demo project for Spring Boot for Reactive</description> - <url/> - <licenses> - <license/> - </licenses> - <developers> - <developer/> - </developers> - <scm> - <connection/> - <developerConnection/> - <tag/> - <url/> - </scm>
38-61: Organize trace dependencies more efficientlyThe trace dependencies section contains multiple empty lines and redundant comments. Consider organizing this section more efficiently and providing more detailed comments about what each dependency provides.
</dependency> - - <!--Trace--> - + <!-- Tracing Dependencies --> <!-- Micrometer Tracing with OpenTelemetry bridge --> <dependency> <groupId>io.micrometer</groupId> <artifactId>micrometer-tracing-bridge-otel</artifactId> </dependency> <!-- OpenTelemetry SDK --> <dependency> <groupId>io.opentelemetry</groupId> <artifactId>opentelemetry-sdk</artifactId> </dependency> <!-- Logging exporter to print spans to console --> <dependency> <groupId>io.opentelemetry</groupId> <artifactId>opentelemetry-exporter-logging</artifactId> </dependency> - - <!--End of Trace--> - -
100-106: Specify Lombok version in annotationProcessorPathsThe Maven compiler plugin configuration includes Lombok as an annotation processor but doesn't specify its version. It's a good practice to explicitly set the version to ensure build stability.
<configuration> <annotationProcessorPaths> <path> <groupId>org.projectlombok</groupId> <artifactId>lombok</artifactId> + <version>${lombok.version}</version> </path> </annotationProcessorPaths> </configuration>reactive/src/main/java/com/example/reactive/sample/TracingConfig.java (1)
36-39: Remove or document commented codeThere's commented-out code showing an alternative tracer implementation. This should either be removed if it's not needed or documented with a comment explaining why it's kept.
-// @Bean -// public Tracer tracer(OpenTelemetry openTelemetry) { -// return MicrometerTracerFactory.create(openTelemetry); -// }Or if it needs to be kept for reference:
+ // Alternative implementation using MicrometerTracerFactory + // Left as reference for when using a custom OpenTelemetry provider // @Bean // public Tracer tracer(OpenTelemetry openTelemetry) { // return MicrometerTracerFactory.create(openTelemetry); // }reactive/src/main/java/com/example/reactive/BlogService.java (1)
41-43:deleteBlogignores the “not found” scenario
deleteById(id)completes successfully even when no record existed, which eventually yields204/200to the caller.
Consider verifying existence first (or usingrepository.delete(...)with the entity) and converting the empty case to404exactly as suggested forupdateBlog.reactive/src/main/java/com/example/reactive/BlogController.java (1)
41-44: Return 204 No Content for successful deleteAfter a successful delete, returning a body is unnecessary. Consider:
@DeleteMapping("/{id}") public Mono<ResponseEntity<Void>> deleteBlog(@PathVariable String id) { return blogService.deleteBlog(id) .then(Mono.just(ResponseEntity.noContent().build())); }reactive/src/main/java/com/example/reactive/EpisodeController.java (3)
32-72: Remove large commented‑out block or move to docs/testsThe legacy span‑management implementation clutters the file and risks bit‑rot. Keeping ~40 lines of dead code inside production classes hurts readability.
If you still need the alternative example, place it in:
docs/as a snippet, or- a dedicated
EpisodeControllerManualSpanExample.javaexcluded from the build.
86-89: Beware ofcollectList()memory pressure
collectList()materialises the full response before sending it, which defeats back‑pressure and can blow memory for large datasets. If the downstream consumer can handle a stream, prefer returningFlux<CustomEpisode>directly.
108-129:callApiWithTraceduplicates injected dependencies & is unusedThe method accepts
WebClientandTracerparameters even though the class already has fields for them, and no endpoint exposes it.• Remove the parameters and use the injected beans, or
• Delete / relocate the method if it’s only for unit tests.reactive/mvnw (1)
108-116: Supply‑chain hardening: commit the SHA‑256 checksumThe wrapper honours a
distributionSha256Sumvalue, but none is provided in the generatedmaven-wrapper.properties.
Without it, the download is accepted purely on TLS trust, which leaves the build vulnerable to a MITM attack if the distribution server is compromised.Action items:
- Generate the checksum for the chosen Maven distribution.
- Add a line such as
distributionSha256Sum=<calculated‑sha256>
to.mvn/wrapper/maven-wrapper.properties.Doing so activates the verification block starting at line 222 and will abort the build on checksum mismatch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
reactive/.gitattributes(1 hunks)reactive/.gitignore(1 hunks)reactive/.mvn/wrapper/maven-wrapper.properties(1 hunks)reactive/mvnw(1 hunks)reactive/mvnw.cmd(1 hunks)reactive/pom.xml(1 hunks)reactive/src/main/java/com/example/reactive/Blog.java(1 hunks)reactive/src/main/java/com/example/reactive/BlogController.java(1 hunks)reactive/src/main/java/com/example/reactive/BlogRepository.java(1 hunks)reactive/src/main/java/com/example/reactive/BlogService.java(1 hunks)reactive/src/main/java/com/example/reactive/EpisodeController.java(1 hunks)reactive/src/main/java/com/example/reactive/ReactiveApplication.java(1 hunks)reactive/src/main/java/com/example/reactive/sample/CustomEpisode.java(1 hunks)reactive/src/main/java/com/example/reactive/sample/EpisodeResponse.java(1 hunks)reactive/src/main/java/com/example/reactive/sample/TracingConfig.java(1 hunks)reactive/src/main/java/com/example/reactive/sample/WebClientConfig.java(1 hunks)reactive/src/main/resources/application.yml(1 hunks)reactive/src/test/java/com/example/reactive/ReactiveApplicationTests.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
reactive/src/main/java/com/example/reactive/sample/WebClientConfig.java (1)
reactive/src/main/java/com/example/reactive/sample/TracingConfig.java (1)
Configuration(13-40)
reactive/src/main/java/com/example/reactive/sample/TracingConfig.java (1)
reactive/src/main/java/com/example/reactive/sample/WebClientConfig.java (1)
Configuration(7-14)
reactive/src/main/java/com/example/reactive/EpisodeController.java (2)
reactive/src/main/java/com/example/reactive/sample/CustomEpisode.java (1)
CustomEpisode(3-17)reactive/src/main/java/com/example/reactive/sample/EpisodeResponse.java (1)
EpisodeResponse(3-9)
reactive/src/main/java/com/example/reactive/BlogController.java (1)
reactive/src/main/java/com/example/reactive/EpisodeController.java (1)
RestController(23-131)
🔇 Additional comments (8)
reactive/.gitattributes (1)
1-2: Properly normalize line endings
The.gitattributesfile correctly enforces LF for themvnwshell script and CRLF for Windows.cmdfiles, ensuring consistent behavior across platforms.reactive/.mvn/wrapper/maven-wrapper.properties (1)
17-19: Verify non-standarddistributionTypeusage
ThedistributionType=only-scriptproperty isn’t commonly documented for Maven Wrapper configurations. Please confirm that the wrapper scripts (v3.3.2) recognize and handle this setting as intended, so Maven is downloaded and executed correctly.reactive/.gitignore (1)
1-34: Review: .gitignore configuration looks solid
The new.gitignoreeffectively excludes build artifacts, IDE files, and preserves necessary wrapper and module outputs. No changes needed at this time.reactive/src/main/java/com/example/reactive/ReactiveApplication.java (1)
1-14: Application entry point is correctly defined
The@SpringBootApplicationannotation andmainmethod launch the reactive Spring Boot context as expected. This class meets the standard conventions for bootstrapping the application.reactive/src/main/java/com/example/reactive/Blog.java (1)
8-16:Details
❓ Verification inconclusive
Add validation constraints and consider using auditing for timestamp
The
Blogentity lacks validation constraints to ensure data integrity. Also, thecreatedAtfield is initialized at object creation time rather than persistence time. Consider using Spring Data's auditing features for proper timestamp management.+import org.springframework.data.annotation.CreatedDate; +import javax.validation.constraints.NotBlank; +import javax.validation.constraints.Size; @Data @Document(collection = "blogs") +// Add this and configure auditing in your application +// @EntityListeners(AuditingEntityListener.class) public class Blog { @Id private String id; + @NotBlank + @Size(max = 100) private String title; + @NotBlank private String content; + @NotBlank private String author; - private LocalDateTime createdAt = LocalDateTime.now(); + @CreatedDate + private LocalDateTime createdAt; }Is Spring Data MongoDB auditing configured in the application? Let's check:
🏁 Script executed:
#!/bin/bash rg -A 3 'EnableMongoAuditing|MongoConfig' --type javaLength of output: 55
Add validation constraints and enable MongoDB auditing
The
Blogentity currently:
- Has no JSR‑303 validation on
title,content, orauthor.- Initializes
createdAtat object instantiation rather than at persist time.Apply bean validation and Spring Data auditing so that timestamps are set when the document is saved.
File: reactive/src/main/java/com/example/reactive/Blog.java
@@ public class Blog { - private String title; + @NotBlank + @Size(max = 100) + private String title; - private String content; + @NotBlank + private String content; - private String author; + @NotBlank + private String author; - private LocalDateTime createdAt = LocalDateTime.now(); + @CreatedDate + private LocalDateTime createdAt; }Don’t forget to import the annotations:
import javax.validation.constraints.NotBlank; import javax.validation.constraints.Size; import org.springframework.data.annotation.CreatedDate;And ensure auditing is enabled in your application, for example:
@Configuration @EnableReactiveMongoAuditing public class MongoConfig { }or, for blocking MongoDB repositories:
@Configuration @EnableMongoAuditing public class MongoConfig { }Please verify that you have the appropriate
@EnableReactiveMongoAuditingor@EnableMongoAuditingin your config so that@CreatedDateis applied.reactive/pom.xml (1)
6-9:Details
❓ Verification inconclusive
Verify Spring Boot version compatibility
The project uses Spring Boot 3.4.3, which appears to be a very recent or future version that might cause compatibility issues with other dependencies or environments. As of early 2024, Spring Boot 3.2.x was the latest GA release.
🌐 Web query:
What is the latest GA version of Spring Boot as of April 2024?💡 Result:
Latest GA Version of Spring Boot as of April 2024
The latest General Availability (GA) version of Spring Boot as of April 2024 is Spring Boot 3.4.0.
Supporting Details
- Spring Boot 3.4.0 was officially released in November 2024, bringing new features such as structured logging, expanded support for virtual threads, and several dependency upgrades[2][5].
- The Spring News Roundup from November 2024 specifically highlights the GA release of Spring Boot 3.4.0[2].
- The release notes and Spring ecosystem blogs corroborate that 3.4.x is the current major line, with service releases (e.g., 3.4.4) following throughout early 2025[1][4][7][10].
- As of March 2025, the most current patch release is Spring Boot 3.4.4[7][9][10], meaning the entire 3.4.x series was available starting from 3.4.0 at the close of 2024.
Summary Table
Date Latest Spring Boot GA Version April 2024 3.4.0 March 2025 3.4.4 As of April 2024, the latest GA version of Spring Boot is 3.4.0, initiating the 3.4.x release line. Service releases such as 3.4.4 followed in 2025, but 3.4.0 is the relevant GA version for April 2024.[2][5][7]
If you are starting a new project or planning an upgrade as of April 2024, Spring Boot 3.4.0 would be the recommended GA version.
Citations:
- 1: https://spring.io/blog/category/releases
- 2: https://www.infoq.com/news/2024/11/spring-news-roundup-nov18-2024/
- 3: https://spring.io/blog/2024/04/24/this-week-in-spring-tuesday-april-23rd-2024/
- 4: https://spring.io/projects/spring-boot
- 5: https://www.infoq.com/news/2024/11/spring-6-2-spring-boot-3-4/
- 6: https://mvnrepository.com/artifact/org.springframework.boot/spring-boot
- 7: https://www.codejava.net/frameworks/spring-boot/spring-boot-version-history
- 8: https://blogs.vmware.com/tanzu/these-months-in-spring-may-and-june-2024/
- 9: https://endoflife.date/spring-boot
- 10: https://bootify.io/spring-boot/spring-boot-latest-version.html
Confirm support for Spring Boot 3.4.x
As of April 2024, the latest GA release was Spring Boot 3.4.0 (Nov 2024), and 3.4.3 is a later patch in that line. Please verify that all your dependencies, plugins and deployment environments are compatible with the 3.4.x series; if not, consider sticking to the most recent 3.2.x GA release.
• File: reactive/pom.xml (lines 6–9)
<groupId>org.springframework.boot</groupId> <artifactId>spring-boot-starter-parent</artifactId> <version>3.4.3</version> <relativePath/> <!-- lookup parent from repository -->reactive/mvnw.cmd (1)
1-150: Standard Maven wrapper – no issuesThe script matches the stock Maven Wrapper 3.3.2 and requires no review action.
reactive/mvnw (1)
1-1: Ensure the script is committed with executable permissionUnix environments depend on the
+xbit for wrapper scripts.
Please verify thatreactive/mvnwis marked as executable (chmod 755 reactive/mvnw) so that CI/CD runners and local checkouts can invoke./mvnwwithout manual intervention.
| @Configuration | ||
| public class WebClientConfig { | ||
|
|
||
| @Bean | ||
| public WebClient webClient(WebClient.Builder builder) { | ||
| return builder.build(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance WebClient configuration with timeouts and error handling
The current WebClient configuration is minimal. For a more robust implementation, consider adding timeouts, error handling, and integration with the tracing system.
@Configuration
public class WebClientConfig {
@Bean
- public WebClient webClient(WebClient.Builder builder) {
- return builder.build();
+ public WebClient webClient(WebClient.Builder builder, Tracer tracer) {
+ return builder
+ .codecs(configurer -> configurer.defaultCodecs().maxInMemorySize(16 * 1024 * 1024))
+ .filter((request, next) -> next.exchange(request)
+ .timeout(Duration.ofSeconds(10))
+ .onErrorResume(e -> {
+ if (e instanceof TimeoutException) {
+ return Mono.error(new RuntimeException("API request timed out", e));
+ }
+ return Mono.error(e);
+ }))
+ .build();
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Configuration | |
| public class WebClientConfig { | |
| @Bean | |
| public WebClient webClient(WebClient.Builder builder) { | |
| return builder.build(); | |
| } | |
| @Configuration | |
| public class WebClientConfig { | |
| @Bean | |
| public WebClient webClient(WebClient.Builder builder, Tracer tracer) { | |
| return builder | |
| .codecs(configurer -> configurer.defaultCodecs().maxInMemorySize(16 * 1024 * 1024)) | |
| .filter((request, next) -> next.exchange(request) | |
| .timeout(Duration.ofSeconds(10)) | |
| .onErrorResume(e -> { | |
| if (e instanceof TimeoutException) { | |
| return Mono.error(new RuntimeException("API request timed out", e)); | |
| } | |
| return Mono.error(e); | |
| })) | |
| .build(); | |
| } | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@developer-shubham101 Great! I see the WebClient has been enhanced with timeout handling, error management, and increased memory allocation. These improvements will make your reactive application more robust.
| @SpringBootTest | ||
| class ReactiveApplicationTests { | ||
|
|
||
| @Test | ||
| void contextLoads() { | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Expand test coverage beyond context loading
The current test only verifies that the Spring context loads successfully. Consider adding more comprehensive tests for your reactive components.
Add integration tests for your reactive endpoints and repositories. For example:
@SpringBootTest
@AutoConfigureWebTestClient
class ReactiveApplicationTests {
@Autowired
private WebTestClient webTestClient;
@Autowired
private BlogRepository blogRepository;
@Test
void contextLoads() {
}
@Test
void shouldReturnBlogsForExistingAuthor() {
// Given
String author = "Test Author";
Blog blog = new Blog();
blog.setAuthor(author);
blog.setTitle("Test Blog");
blog.setContent("Test Content");
blogRepository.save(blog)
.block();
// When/Then
webTestClient.get()
.uri("/blogs/author/{author}", author)
.exchange()
.expectStatus().isOk()
.expectBodyList(Blog.class)
.hasSize(1)
.contains(blog);
}
}
reactive/src/main/java/com/example/reactive/sample/CustomEpisode.java
Outdated
Show resolved
Hide resolved
| SdkTracerProvider sdkTracerProvider = SdkTracerProvider.builder() | ||
| .addSpanProcessor(SimpleSpanProcessor.create(new LoggingSpanExporter())) | ||
| .build(); | ||
|
|
||
| OpenTelemetrySdk openTelemetry = OpenTelemetrySdk.builder() | ||
| .setTracerProvider(sdkTracerProvider) | ||
| .build(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider using BatchSpanProcessor for production environments
The current configuration uses a SimpleSpanProcessor which processes spans synchronously on the same thread as the application. For production environments, a BatchSpanProcessor would be more efficient as it batches spans and exports them asynchronously.
SdkTracerProvider sdkTracerProvider = SdkTracerProvider.builder()
- .addSpanProcessor(SimpleSpanProcessor.create(new LoggingSpanExporter()))
+ .addSpanProcessor(BatchSpanProcessor.builder(new LoggingSpanExporter())
+ .setScheduleDelay(1, TimeUnit.SECONDS)
+ .build())
.build();Don't forget to add the missing import:
+import io.opentelemetry.sdk.trace.export.BatchSpanProcessor;
+import java.util.concurrent.TimeUnit;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SdkTracerProvider sdkTracerProvider = SdkTracerProvider.builder() | |
| .addSpanProcessor(SimpleSpanProcessor.create(new LoggingSpanExporter())) | |
| .build(); | |
| OpenTelemetrySdk openTelemetry = OpenTelemetrySdk.builder() | |
| .setTracerProvider(sdkTracerProvider) | |
| .build(); | |
| // add these imports | |
| import io.opentelemetry.sdk.trace.export.BatchSpanProcessor; | |
| import java.util.concurrent.TimeUnit; | |
| // … | |
| SdkTracerProvider sdkTracerProvider = SdkTracerProvider.builder() | |
| - .addSpanProcessor(SimpleSpanProcessor.create(new LoggingSpanExporter())) | |
| + .addSpanProcessor(BatchSpanProcessor.builder(new LoggingSpanExporter()) | |
| + .setScheduleDelay(1, TimeUnit.SECONDS) | |
| + .build()) | |
| .build(); | |
| OpenTelemetrySdk openTelemetry = OpenTelemetrySdk.builder() | |
| .setTracerProvider(sdkTracerProvider) | |
| .build(); |
| @GetMapping("/{id}") | ||
| public Mono<Blog> getBlogById(@PathVariable String id) { | ||
| return blogService.getBlogById(id); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Propagate 404 when a blog is missing
getBlogById may emit an empty Mono, which WebFlux will serialise as null with a 200 status. Wrap result in ResponseEntity and convert Mono.empty() to 404:
@GetMapping("/{id}")
public Mono<ResponseEntity<Blog>> getBlogById(@PathVariable String id) {
return blogService.getBlogById(id)
.map(ResponseEntity::ok)
.switchIfEmpty(Mono.just(ResponseEntity.notFound().build()));
}Repeat the same pattern for updateBlog and deleteBlog.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
package com.example.reactive;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.*;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import java.net.URI;
@RestController
@RequestMapping("/blogs")
public class BlogController {
private final BlogService blogService;
public BlogController(BlogService blogService) {
this.blogService = blogService;
}
@PostMapping
public Mono<ResponseEntity<Blog>> createBlog(@RequestBody Blog blog) {
return blogService.createBlog(blog)
.map(saved -> ResponseEntity
.created(URI.create("/blogs/" + saved.getId()))
.body(saved));
}
@GetMapping
public Flux<Blog> getAllBlogs() {
return blogService.getAllBlogs();
}
@GetMapping("/{id}")
public Mono<ResponseEntity<Blog>> getBlogById(@PathVariable String id) {
return blogService.getBlogById(id)
.map(ResponseEntity::ok)
.switchIfEmpty(Mono.just(ResponseEntity.notFound().build()));
}
@GetMapping("/author/{author}")
public Flux<Blog> getBlogsByAuthor(@PathVariable String author) {
return blogService.getBlogsByAuthor(author);
}
@PutMapping("/{id}")
public Mono<Blog> updateBlog(@PathVariable String id, @RequestBody Blog blog) {
return blogService.updateBlog(id, blog);
}
@DeleteMapping("/{id}")
public Mono<Void> deleteBlog(@PathVariable String id) {
return blogService.deleteBlog(id);
}
}
@developer-shubham101 I see you've fixed the getBlogById method to properly handle 404 responses. Great job!
For even better API consistency, you might also want to update the updateBlog method similarly:
@PutMapping("/{id}")
public Mono<ResponseEntity<Blog>> updateBlog(@PathVariable String id, @RequestBody Blog blog) {
return blogService.updateBlog(id, blog)
.map(ResponseEntity::ok)
.switchIfEmpty(Mono.just(ResponseEntity.notFound().build()));
}This would ensure a consistent response pattern across your API endpoints when a resource isn't found.
🏁 Script executed:
#!/bin/bash
# Get the current state of the BlogController.java file
cat reactive/src/main/java/com/example/reactive/BlogController.java
Length of output: 1652
@developer-shubham101 I see you've fixed the getBlogById method to properly handle 404 responses. Great job!
For even better API consistency, you might also want to update the updateBlog method similarly:
@PutMapping("/{id}")
public Mono<ResponseEntity<Blog>> updateBlog(@PathVariable String id, @RequestBody Blog blog) {
return blogService.updateBlog(id, blog)
.map(ResponseEntity::ok)
.switchIfEmpty(Mono.just(ResponseEntity.notFound().build()));
}This would ensure a consistent response pattern across your API endpoints when a resource isn't found.
🧬 Code Graph Analysis Results
Possibly relevant snippets from reactive/src/main/java/com/example/reactive/EpisodeController.java (lines 22-130):
@RestController
@RequestMapping("/episodes")
@RequiredArgsConstructor
public class EpisodeController {
private final WebClient webClient;
private final Tracer tracer; // from micrometer
private final Logger logger = LoggerFactory.getLogger(EpisodeController.class);
@GetMapping
public Mono<ResponseEntity<List<CustomEpisode>>> getEpisodes() {
String traceId = tracer.currentSpan() != null
? tracer.currentSpan().context().traceId()
: "N/A";
return webClient.get()
.uri("https://apissa.sampleapis.com/futurama/episodexxs") // sample typo in URL to trigger error
.retrieve()
.bodyToFlux(EpisodeResponse.class)
.contextWrite(Context.of("traceId", traceId))
.map(ep -> new CustomEpisode(ep.getTitle(), ep.getWriters(), ep.getOriginalAirDate(), ep.getDesc(), ep.getId()))
.collectList()
.map(ResponseEntity::ok)
.onErrorResume(e ->
Mono.deferContextual(ctx -> {
String ctxTraceId = ctx.getOrDefault("traceId", "N/A");
logger.info("TraceId inside context: {}", ctxTraceId);
logger.error("Error fetching episodes. Trace ID: {}", ctxTraceId, e);
CustomEpisode errorEpisode = new CustomEpisode(
"Error occurred", "", "", "Trace ID: " + traceId + ", error: " + e.getMessage(), -1
);
return Mono.just(ResponseEntity
.status(HttpStatus.INTERNAL_SERVER_ERROR)
.body(Collections.singletonList(errorEpisode)));
})
);
}
public Mono<ResponseEntity<String>> callApiWithTrace(WebClient webClient, Tracer tracer) {
String traceId = tracer.currentSpan() != null
? tracer.currentSpan().context().traceId()
: "N/A";
return webClient.get()
.uri("https://api.sampleapis.com/futurama/episodes")
.retrieve()
.bodyToMono(String.class)
.contextWrite(Context.of("traceId", traceId))
.flatMap(response -> Mono.deferContextual(ctx -> {
String ctxTraceId = ctx.getOrDefault("traceId", "N/A");
logger.info("TraceId inside context: {}", ctxTraceId);
return Mono.just(ResponseEntity.ok("Success with traceId: " + ctxTraceId));
}))
.onErrorResume(e -> Mono.deferContextual(ctx -> {
String ctxTraceId = ctx.getOrDefault("traceId", "N/A");
logger.error("Error with traceId {}: {}", ctxTraceId, e.getMessage());
return Mono.just(ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
.body("Error: " + e.getMessage() + " TraceId: " + ctxTraceId));
}));
}
}Summary:
- This controller handles reactive HTTP requests for episodes.
- It uses
WebClientto call external APIs andTracerfor distributed tracing. - The
getEpisodes()method demonstrates error handling with reactive context propagation of trace IDs, logging errors with trace information, and returning a fallback error response wrapped in aMono<ResponseEntity<List<CustomEpisode>>>. - The
callApiWithTrace()method similarly demonstrates making an API call with trace context propagation and error handling. - Both methods use
contextWriteandMono.deferContextualto access and log trace IDs from Reactor's context. - This snippet is relevant for understanding reactive controller patterns, error handling, and context propagation, which may relate to the
BlogControllerreactive patterns.
✏️ Learnings added
Learnt from: developer-shubham101
PR: developer-shubham101/SpringBootSample#1
File: reactive/src/main/java/com/example/reactive/BlogController.java:0-0
Timestamp: 2025-04-17T18:06:33.774Z
Learning: In reactive Spring Boot controllers, wrap method returns in ResponseEntity and use switchIfEmpty to handle empty Mono cases with proper status codes (e.g., 404 Not Found) for better API consistency.
Learnt from: developer-shubham101
PR: developer-shubham101/SpringBootSample#1
File: reactive/src/main/java/com/example/reactive/BlogController.java:0-0
Timestamp: 2025-04-17T18:06:33.774Z
Learning: In reactive Spring Boot controllers, wrap method returns in ResponseEntity and use switchIfEmpty to handle empty Mono cases with proper status codes (e.g., 404 Not Found) for better API consistency.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| .map(ep -> new CustomEpisode(ep.title, ep.writers, ep.originalAirDate, ep.desc, ep.id)) | ||
| .collectList() | ||
| .map(ResponseEntity::ok) | ||
| .onErrorResume(e -> | ||
| Mono.deferContextual(ctx -> { | ||
| String ctxTraceId = ctx.getOrDefault("traceId", "N/A"); | ||
| logger.info("TraceId inside context: {}", ctxTraceId); | ||
|
|
||
| logger.error("Error fetching episodes. Trace ID: {}", traceId, e); | ||
|
|
||
| CustomEpisode errorEpisode = new CustomEpisode( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use contextual traceId consistently in error log & response
Inside onErrorResume you log with the outer‑scoped traceId, but you just derived a possibly different ctxTraceId. To avoid mismatches:
-logger.error("Error fetching episodes. Trace ID: {}", traceId, e);
+logger.error("Error fetching episodes. Trace ID: {}", ctxTraceId, e);And construct the errorEpisode with the same ctxTraceId.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@developer-shubham101 Great! Thank you for confirming the fix. Using the contextual trace ID consistently will help maintain proper correlation between logs and error responses in your reactive application.
| set -euf | ||
| [ "${MVNW_VERBOSE-}" != debug ] || set -x | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider adding pipefail to strengthen error handling
The script already uses set -euf, which exits on the first failing command, treats unset variables as errors, and prevents filename globbing surprises.
Adding the -o pipefail option ensures that a failure in any part of a command pipeline is detected—otherwise only the status of the last command in the pipeline is considered.
-set -euf
+set -euo pipefailThis small change further reduces the risk of silently ignoring partial failures (e.g., failed downloads that are piped through grep/awk later).
Committable suggestion skipped: line range outside the PR's diff.
Docstrings generation was requested by @developer-shubham101. * #1 (comment) The following files were modified: * `reactive/src/main/java/com/example/reactive/BlogController.java` * `reactive/src/main/java/com/example/reactive/BlogRepository.java` * `reactive/src/main/java/com/example/reactive/BlogService.java` * `reactive/src/main/java/com/example/reactive/EpisodeController.java` * `reactive/src/main/java/com/example/reactive/ReactiveApplication.java` * `reactive/src/main/java/com/example/reactive/sample/CustomEpisode.java` * `reactive/src/main/java/com/example/reactive/sample/TracingConfig.java` * `reactive/src/main/java/com/example/reactive/sample/WebClientConfig.java`
|
Note Generated docstrings for this pull request at #2 |
…nfig.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
reactive/src/main/java/com/example/reactive/sample/WebClientConfig.java (1)
24-26:⚠️ Potential issueRemove extra closing brace
There's an extra closing brace at the end of the file that causes a syntax error.
Fix the syntax error by removing the extra closing brace:
} } -}
♻️ Duplicate comments (1)
reactive/src/main/java/com/example/reactive/sample/WebClientConfig.java (1)
11-23: Enhance WebClient configuration with timeouts and error handlingThe WebClient configuration includes appropriate timeout and error handling as recommended in the previous review.
🧹 Nitpick comments (1)
reactive/src/main/java/com/example/reactive/sample/WebClientConfig.java (1)
11-23: Unused Tracer parameterThe
tracerparameter is injected but not utilized in the method. If you're implementing distributed tracing, consider adding tracing to your WebClient requests.Consider using the tracer to add spans to your WebClient requests:
@Bean public WebClient webClient(WebClient.Builder builder, Tracer tracer) { return builder .codecs(configurer -> configurer.defaultCodecs().maxInMemorySize(16 * 1024 * 1024)) .filter((request, next) -> next.exchange(request) .timeout(Duration.ofSeconds(10)) .onErrorResume(e -> { if (e instanceof TimeoutException) { return Mono.error(new RuntimeException("API request timed out", e)); } return Mono.error(e); })) + .filter((request, next) -> { + String spanName = "WebClient " + request.method() + " " + request.url().toString(); + return Mono.deferContextual(ctx -> + Mono.just(tracer.nextSpan().name(spanName)) + .flatMap(span -> { + span.start(); + return next.exchange(request) + .doFinally(signalType -> span.end()); + }) + ); + }) .build(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
reactive/src/main/java/com/example/reactive/sample/WebClientConfig.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
reactive/src/main/java/com/example/reactive/sample/WebClientConfig.java (1)
reactive/src/main/java/com/example/reactive/sample/TracingConfig.java (1)
Configuration(13-40)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
reactive/src/main/java/com/example/reactive/EpisodeController.java (1)
89-102: Use contextual traceId consistently in error log & response.Inside
onErrorResumeyou log with the context-derivedctxTraceId, but you construct theerrorEpisodewith the outer-scopedtraceId. To avoid mismatches:- CustomEpisode errorEpisode = new CustomEpisode( - "Error occurred", "", "", "Trace ID: " + traceId + ", error: " + e.getMessage(), -1 - ); + CustomEpisode errorEpisode = new CustomEpisode( + "Error occurred", "", "", "Trace ID: " + ctxTraceId + ", error: " + e.getMessage(), -1 + );reactive/src/main/java/com/example/reactive/BlogController.java (2)
49-52:⚠️ Potential issueAdd 404 handling for deleteBlog
The deleteBlog method should also return a 404 status when the blog doesn't exist.
@DeleteMapping("/{id}") -public Mono<Void> deleteBlog(@PathVariable String id) { - return blogService.deleteBlog(id); +public Mono<ResponseEntity<Void>> deleteBlog(@PathVariable String id) { + return blogService.findById(id) + .flatMap(blog -> blogService.deleteBlog(id) + .then(Mono.just(ResponseEntity.noContent().<Void>build()))) + .switchIfEmpty(Mono.just(ResponseEntity.notFound().build())); }
44-47:⚠️ Potential issueAdd 404 handling for updateBlog
The updateBlog method doesn't handle the case when the blog with the specified ID doesn't exist. Apply the same pattern as in getBlogById.
@PutMapping("/{id}") -public Mono<Blog> updateBlog(@PathVariable String id, @RequestBody Blog blog) { - return blogService.updateBlog(id, blog); +public Mono<ResponseEntity<Blog>> updateBlog(@PathVariable String id, @RequestBody Blog blog) { + return blogService.updateBlog(id, blog) + .map(ResponseEntity::ok) + .switchIfEmpty(Mono.just(ResponseEntity.notFound().build())); }
🧹 Nitpick comments (5)
reactive/src/main/java/com/example/reactive/EpisodeController.java (2)
31-71: Consider removing or documenting commented code.While commented code can serve as a reference, it's generally better to remove it or clearly document why it's being kept. If this is meant as an educational example of manual span creation, consider adding a comment explaining its purpose and how it differs from the active implementation.
107-128: Consider refactoring callApiWithTrace method for better reusability.This helper method accepts
webClientandtraceras parameters, but these are already available as instance fields. Consider:
- Using the class fields instead of parameters if this is meant to be used within this controller
- Making it a static utility method in a separate class if meant to be used across controllers
- Adding JavaDoc to document its purpose and usage
- public Mono<ResponseEntity<String>> callApiWithTrace(WebClient webClient, Tracer tracer) { + /** + * Helper method to call an API with trace context propagation. + * + * @return A Mono containing the response with trace information + */ + public Mono<ResponseEntity<String>> callApiWithTrace() { + // Use the instance fields instead of parametersreactive/src/main/java/com/example/reactive/BlogController.java (3)
27-30: Consider pagination for getAllBlogsWhile the implementation is correct, returning all blogs without pagination could lead to performance issues as the dataset grows.
@GetMapping -public Flux<Blog> getAllBlogs() { - return blogService.getAllBlogs(); +public Flux<Blog> getAllBlogs( + @RequestParam(defaultValue = "0") int page, + @RequestParam(defaultValue = "20") int size) { + return blogService.getAllBlogs(page, size); }
39-42: Consider pagination and empty result handling for author-specific blogsSimilar to getAllBlogs, this endpoint could benefit from pagination. Also, consider returning a 404 if no blogs are found for the specified author, or an empty collection with a 200 status depending on your API design.
@GetMapping("/author/{author}") -public Flux<Blog> getBlogsByAuthor(@PathVariable String author) { - return blogService.getBlogsByAuthor(author); +public Mono<ResponseEntity<Flux<Blog>>> getBlogsByAuthor( + @PathVariable String author, + @RequestParam(defaultValue = "0") int page, + @RequestParam(defaultValue = "20") int size) { + Flux<Blog> blogs = blogService.getBlogsByAuthor(author, page, size); + return blogs.hasElements() + .map(hasElements -> hasElements + ? ResponseEntity.ok(blogs) + : ResponseEntity.notFound().build()); }
1-53: Consider adding validation for request bodiesThe controller doesn't include validation for blog objects received in POST and PUT requests. Consider adding validation to ensure data integrity.
// Add these imports +import org.springframework.validation.annotation.Validated; +import javax.validation.Valid; // Add @Validated to the controller class -@RestController +@RestController +@Validated @RequestMapping("/blogs") public class BlogController { // ... existing code ... // Add @Valid to request bodies @PostMapping - public Mono<ResponseEntity<Blog>> createBlog(@RequestBody Blog blog) { + public Mono<ResponseEntity<Blog>> createBlog(@Valid @RequestBody Blog blog) { // ... existing code ... } @PutMapping("/{id}") - public Mono<Blog> updateBlog(@PathVariable String id, @RequestBody Blog blog) { + public Mono<ResponseEntity<Blog>> updateBlog(@PathVariable String id, @Valid @RequestBody Blog blog) { // ... existing code with 404 handling ... } }Then add validation annotations to the Blog class fields (e.g., @notblank, @SiZe).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
reactive/src/main/java/com/example/reactive/BlogController.java(1 hunks)reactive/src/main/java/com/example/reactive/BlogService.java(1 hunks)reactive/src/main/java/com/example/reactive/EpisodeController.java(1 hunks)reactive/src/main/java/com/example/reactive/sample/EpisodeResponse.java(1 hunks)reactive/src/main/java/com/example/reactive/sample/WebClientConfig.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- reactive/src/main/java/com/example/reactive/sample/EpisodeResponse.java
- reactive/src/main/java/com/example/reactive/sample/WebClientConfig.java
- reactive/src/main/java/com/example/reactive/BlogService.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
reactive/src/main/java/com/example/reactive/EpisodeController.java (1)
reactive/src/main/java/com/example/reactive/sample/CustomEpisode.java (1)
CustomEpisode(3-17)
🔇 Additional comments (7)
reactive/src/main/java/com/example/reactive/EpisodeController.java (3)
22-30: Well-structured controller class with proper dependency injection.The controller is well organized with appropriate request mapping and dependency injection using Lombok's
@RequiredArgsConstructor.
80-82: URL contains intentional typo for error testing.The endpoint contains an intentional typo (
episodexxsinstead ofepisodes) to trigger error handling. In a production environment, this should be replaced with the correct URL.Is this intentional error handling test meant to stay in the production code, or should it be moved to a test class?
1-130: Add unit tests for reactive flow and error handling.Since this controller has complex reactive flows and error handling, it would benefit from thorough unit testing, especially testing:
- Successful API responses
- Error handling paths
- Trace ID propagation through reactive contexts
Consider using WebTestClient and MockWebServer to simulate both successful and error responses.
reactive/src/main/java/com/example/reactive/BlogController.java (4)
1-9: Import structure looks goodThe imports are well organized and include all necessary classes for a reactive REST controller.
10-17: Controller structure follows best practicesGood use of constructor-based dependency injection and immutable fields. The RequestMapping path is appropriate for a RESTful API.
19-25: Excellent implementation of POST endpointThe createBlog method properly returns 201 Created with a Location header pointing to the new resource. This follows REST best practices and helps clients discover the URL of the newly created resource.
32-37: Proper empty result handlingGood implementation of 404 handling when a blog is not found, using switchIfEmpty to return a not found response.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (1)
reactive/src/main/java/com/example/reactive/BlogService.java (1)
33-43: Good implementation of update with proper error handlingThe update method correctly handles the case when a blog is not found by returning a 404 error. This approach is consistent with RESTful API design principles.
🧹 Nitpick comments (1)
reactive/src/main/java/com/example/reactive/BlogService.java (1)
17-19: Consider adding validation for blog creationThe
createBlogmethod doesn't validate the input blog object. Adding validation would prevent saving invalid data.public Mono<Blog> createBlog(Blog blog) { + // Validate required fields + if (blog.getTitle() == null || blog.getTitle().isEmpty() || + blog.getContent() == null || blog.getContent().isEmpty()) { + return Mono.error(new ResponseStatusException(HttpStatus.BAD_REQUEST, + "Blog title and content are required")); + } return blogRepository.save(blog); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
reactive/src/main/java/com/example/reactive/BlogController.java(1 hunks)reactive/src/main/java/com/example/reactive/BlogService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- reactive/src/main/java/com/example/reactive/BlogController.java
🔇 Additional comments (1)
reactive/src/main/java/com/example/reactive/BlogService.java (1)
9-15: Good practice using constructor-based dependency injectionUsing constructor-based dependency injection is a Spring best practice as it makes dependencies explicit and ensures they are required.
Summary by CodeRabbit