-
Notifications
You must be signed in to change notification settings - Fork 0
Redis #55
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
base: main
Are you sure you want to change the base?
Redis #55
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| FROM maven:3.9.9-eclipse-temurin-24 AS builder | ||
| WORKDIR /build | ||
| COPY pom.xml . | ||
| COPY src ./src | ||
| RUN mvn clean package | ||
|
|
||
| FROM openjdk:24-jdk | ||
| WORKDIR /app | ||
| ARG JAR_FILE=target/springboot25-0.0.1-SNAPSHOT.jar | ||
| COPY --from=builder /build/${JAR_FILE} app.jar | ||
| ENTRYPOINT ["java","-jar","/app/app.jar"] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,8 @@ | ||
| services: | ||
| redis: | ||
| image: 'redis:7-alpine' | ||
| ports: | ||
| - "6379:6379" | ||
|
Comment on lines
+2
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Redis service configuration needs improvements The Redis service is correctly configured but lacks data persistence and security measures. Consider adding:
redis:
image: 'redis:7-alpine'
+ command: redis-server --requirepass ${REDIS_PASSWORD}
ports:
- "6379:6379"
+ volumes:
+ - redis-data:/data
+ healthcheck:
+ test: ["CMD", "redis-cli", "ping"]
+ interval: 5s
+ timeout: 5s
+ retries: 5 |
||
| postgres: | ||
| image: 'postgres:latest' | ||
| container_name: 'postgres-db' | ||
|
|
@@ -9,13 +13,19 @@ services: | |
| ports: | ||
| - '5432:5432' | ||
| app: | ||
| build: | ||
| context: . | ||
| dockerfile: Dockerfile | ||
| image: 'catinder:latest' | ||
| depends_on: | ||
| - redis | ||
| - postgres | ||
| image: 'catinder:latest' | ||
| env_file: | ||
| - .env | ||
| environment: | ||
| - GOOGLE_CLIENT_ID=${GOOGLE_CLIENT_ID} | ||
| - GOOGLE_CLIENT_SECRET=${GOOGLE_CLIENT_SECRET} | ||
| - SPRING_REDIS_HOST=redis | ||
| - SPRING_REDIS_PORT=6379 | ||
|
Comment on lines
+28
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add Redis password configuration If you implement password protection for Redis as suggested above, you'll need to add the password to the application's environment variables. - SPRING_REDIS_HOST=redis
- SPRING_REDIS_PORT=6379
+ - SPRING_REDIS_PASSWORD=${REDIS_PASSWORD}
Comment on lines
+16
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Don't forget to add volume definition If you add the Redis volume as suggested, remember to define it at the bottom of the compose file. ports:
- "8080:8080"
+volumes:
+ redis-data:
|
||
| ports: | ||
| - "8080:8080" | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -51,12 +51,12 @@ | |||||||||||||||||||||||||||||||||||
| <artifactId>flyway-database-postgresql</artifactId> | ||||||||||||||||||||||||||||||||||||
| </dependency> | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| <dependency> | ||||||||||||||||||||||||||||||||||||
| <groupId>org.springframework.boot</groupId> | ||||||||||||||||||||||||||||||||||||
| <artifactId>spring-boot-devtools</artifactId> | ||||||||||||||||||||||||||||||||||||
| <scope>runtime</scope> | ||||||||||||||||||||||||||||||||||||
| <optional>true</optional> | ||||||||||||||||||||||||||||||||||||
| </dependency> | ||||||||||||||||||||||||||||||||||||
| <!-- <dependency>--> | ||||||||||||||||||||||||||||||||||||
| <!-- <groupId>org.springframework.boot</groupId>--> | ||||||||||||||||||||||||||||||||||||
| <!-- <artifactId>spring-boot-devtools</artifactId>--> | ||||||||||||||||||||||||||||||||||||
| <!-- <scope>runtime</scope>--> | ||||||||||||||||||||||||||||||||||||
| <!-- <optional>true</optional>--> | ||||||||||||||||||||||||||||||||||||
| <!-- </dependency>--> | ||||||||||||||||||||||||||||||||||||
| <dependency> | ||||||||||||||||||||||||||||||||||||
| <groupId>org.springframework.boot</groupId> | ||||||||||||||||||||||||||||||||||||
| <artifactId>spring-boot-docker-compose</artifactId> | ||||||||||||||||||||||||||||||||||||
|
|
@@ -148,6 +148,19 @@ | |||||||||||||||||||||||||||||||||||
| <groupId>com.fasterxml.jackson.core</groupId> | ||||||||||||||||||||||||||||||||||||
| <artifactId>jackson-databind</artifactId> | ||||||||||||||||||||||||||||||||||||
| </dependency> | ||||||||||||||||||||||||||||||||||||
| <dependency> | ||||||||||||||||||||||||||||||||||||
| <groupId>org.springframework.boot</groupId> | ||||||||||||||||||||||||||||||||||||
| <artifactId>spring-boot-starter-data-redis</artifactId> | ||||||||||||||||||||||||||||||||||||
| <version>3.1.2</version> | ||||||||||||||||||||||||||||||||||||
| </dependency> | ||||||||||||||||||||||||||||||||||||
| <dependency> | ||||||||||||||||||||||||||||||||||||
| <groupId>org.springframework.boot</groupId> | ||||||||||||||||||||||||||||||||||||
| <artifactId>spring-boot-starter-cache</artifactId> | ||||||||||||||||||||||||||||||||||||
| </dependency> | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+151
to
+159
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Align Redis dependency version with Spring Boot BOM Specifying -<dependency>
- <groupId>org.springframework.boot</groupId>
- <artifactId>spring-boot-starter-data-redis</artifactId>
- <version>3.1.2</version>
-</dependency>
+<dependency>
+ <groupId>org.springframework.boot</groupId>
+ <artifactId>spring-boot-starter-data-redis</artifactId>
+</dependency>📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
| <dependency> | ||||||||||||||||||||||||||||||||||||
| <groupId>org.springframework.boot</groupId> | ||||||||||||||||||||||||||||||||||||
| <artifactId>spring-boot-starter-actuator</artifactId> | ||||||||||||||||||||||||||||||||||||
| </dependency> | ||||||||||||||||||||||||||||||||||||
| </dependencies> | ||||||||||||||||||||||||||||||||||||
| <build> | ||||||||||||||||||||||||||||||||||||
| <plugins> | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| package org.example.springboot25.config; | ||
|
|
||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
| import org.springframework.data.redis.cache.RedisCacheConfiguration; | ||
| import org.springframework.data.redis.cache.RedisCacheManager; | ||
| import org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory; | ||
|
|
||
| import java.time.Duration; | ||
|
|
||
| @Configuration | ||
| public class RedisCacheConfig { | ||
|
|
||
| @Bean | ||
| public RedisCacheManager cacheManager(LettuceConnectionFactory cf) { | ||
| RedisCacheConfiguration config = RedisCacheConfiguration.defaultCacheConfig() | ||
| .entryTtl(Duration.ofMinutes(10)) | ||
| .disableCachingNullValues(); | ||
|
|
||
| return RedisCacheManager.builder(cf) | ||
| .cacheDefaults(config) | ||
| .build(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,14 @@ | ||||||||||
| package org.example.springboot25.dto; | ||||||||||
|
|
||||||||||
| public class CatOutputDTO { | ||||||||||
| import jakarta.persistence.CascadeType; | ||||||||||
| import jakarta.persistence.OneToMany; | ||||||||||
| import org.example.springboot25.entities.CatPhoto; | ||||||||||
|
Comment on lines
+3
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. JPA annotations are incorrectly used in a DTO class. DTOs shouldn't contain JPA annotations like Remove JPA annotations from the DTO class. They should be in the entity class instead: -import jakarta.persistence.CascadeType;
-import jakarta.persistence.OneToMany;
import org.example.springboot25.entities.CatPhoto;📝 Committable suggestion
Suggested change
|
||||||||||
|
|
||||||||||
| import java.io.Serializable; | ||||||||||
| import java.util.ArrayList; | ||||||||||
| import java.util.List; | ||||||||||
|
|
||||||||||
| public class CatOutputDTO implements Serializable { | ||||||||||
| private Long catId; | ||||||||||
| private String catName; | ||||||||||
| private String catProfilePicture; | ||||||||||
|
|
@@ -10,6 +18,17 @@ public class CatOutputDTO { | |||||||||
| private String catPersonality; | ||||||||||
| private Long userId; | ||||||||||
|
|
||||||||||
| @OneToMany(mappedBy = "catPhotoCat", cascade = CascadeType.ALL, orphanRemoval = true) | ||||||||||
| private List<CatPhoto> catPhotos = new ArrayList<>(); | ||||||||||
|
Comment on lines
+21
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. JPA annotation in DTO class will cause runtime errors. The Remove the JPA annotation and simplify the property declaration: - @OneToMany(mappedBy = "catPhotoCat", cascade = CascadeType.ALL, orphanRemoval = true)
private List<CatPhoto> catPhotos = new ArrayList<>();📝 Committable suggestion
Suggested change
|
||||||||||
|
|
||||||||||
| public List<CatPhoto> getCatPhotos() { | ||||||||||
| return catPhotos; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public void setCatPhotos(List<CatPhoto> catPhotos) { | ||||||||||
| this.catPhotos = catPhotos; | ||||||||||
| } | ||||||||||
|
Comment on lines
+24
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainCatPhoto entity is likely not Serializable. If Verify that the 🏁 Script executed: #!/bin/bash
# Check if CatPhoto implements Serializable
grep -A 2 "class CatPhoto" src/main/java/org/example/springboot25/entities/CatPhoto.javaLength of output: 123 CatPhoto must implement Serializable for Redis caching CatPhoto doesn’t implement • File to update:
• Suggested diff: -public class CatPhoto {
+public class CatPhoto implements Serializable {
+ private static final long serialVersionUID = 1L;
|
||||||||||
|
|
||||||||||
| public Long getCatId() { | ||||||||||
| return catId; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,12 +5,14 @@ | |
| import jakarta.validation.constraints.Min; | ||
| import jakarta.validation.constraints.NotBlank; | ||
| import jakarta.validation.constraints.NotNull; | ||
| import org.springframework.data.redis.core.RedisHash; | ||
|
|
||
| import java.io.Serializable; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add serialVersionUID for Serializable implementation While implementing import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;
@Entity
-public class Cat implements Serializable {
+public class Cat implements Serializable {
+ private static final long serialVersionUID = 1L;Also applies to: 15-15 |
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| @Entity | ||
| public class Cat { | ||
| public class Cat implements Serializable { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,6 +15,9 @@ | |||||||||||
| import org.slf4j.Logger; | ||||||||||||
| import org.slf4j.LoggerFactory; | ||||||||||||
| import org.springframework.beans.factory.annotation.Autowired; | ||||||||||||
| import org.springframework.cache.annotation.CacheEvict; | ||||||||||||
| import org.springframework.cache.annotation.CachePut; | ||||||||||||
| import org.springframework.cache.annotation.Cacheable; | ||||||||||||
| import org.springframework.stereotype.Service; | ||||||||||||
| import org.springframework.transaction.annotation.Transactional; | ||||||||||||
| import org.springframework.web.bind.WebDataBinder; | ||||||||||||
|
|
@@ -93,6 +96,7 @@ public Cat updateCat(Long catId, Cat catDetails) { | |||||||||||
| }).orElseThrow(() -> new NotFoundException("Cat not found with id " + catId)); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| @CacheEvict(cacheNames = "deleteCatById", key = "#catId") | ||||||||||||
| public void deleteCatById(Long catId) { | ||||||||||||
|
Comment on lines
+99
to
100
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect cache key configuration for deletion operations. The Align the cache names with the cacheable operations: - @CacheEvict(cacheNames = "deleteCatById", key = "#catId")
+ @CacheEvict(cacheNames = {"catById", "allCats", "allCatsByUser", "allCatsByName"}, allEntries = true)
public void deleteCatById(Long catId) {📝 Committable suggestion
Suggested change
|
||||||||||||
| if (!catRepository.existsById(catId)) { | ||||||||||||
| throw new NotFoundException("Cat not found with id " + catId); | ||||||||||||
|
|
@@ -101,6 +105,7 @@ public void deleteCatById(Long catId) { | |||||||||||
| catRepository.deleteById(catId); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| @CacheEvict(cacheNames = "deleteCat", key = "#catId") | ||||||||||||
| public void deleteCat(Long catId) { | ||||||||||||
|
Comment on lines
+108
to
109
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar cache configuration issue with deleteCat method. The Correct the cache eviction configuration: - @CacheEvict(cacheNames = "deleteCat", key = "#catId")
+ @CacheEvict(cacheNames = {"catById", "allCats", "allCatsByUser", "allCatsByName"}, allEntries = true)
public void deleteCat(Long catId) {📝 Committable suggestion
Suggested change
|
||||||||||||
| deleteCatById(catId); | ||||||||||||
| } | ||||||||||||
|
|
@@ -109,18 +114,22 @@ public void deleteCat(Long catId) { | |||||||||||
| // Externa metoder (DTO) | ||||||||||||
| // ======================== | ||||||||||||
|
|
||||||||||||
| @Cacheable(cacheNames = "allCats") | ||||||||||||
| public List<CatOutputDTO> getAllCats() { | ||||||||||||
| return catRepository.findAll().stream().map(catMapper::toDto).toList(); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| @Cacheable(cacheNames = "allCatsByUser", key = "#user") | ||||||||||||
| public List<CatOutputDTO> getAllCatsByUser(User user) { | ||||||||||||
|
Comment on lines
+122
to
123
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainPotential serialization issue with User parameter in cache key. The Use a more stable identifier for the cache key: - @Cacheable(cacheNames = "allCatsByUser", key = "#user")
+ @Cacheable(cacheNames = "allCatsByUser", key = "#user.userId")
public List<CatOutputDTO> getAllCatsByUser(User user) {Also, verify that the User class implements Serializable properly: 🏁 Script executed: #!/bin/bash
# Check if User implements Serializable
grep -A 2 "class User" src/main/java/org/example/springboot25/entities/User.javaLength of output: 213 Use a stable identifier for the cache key • File: - @Cacheable(cacheNames = "allCatsByUser", key = "#user")
+ @Cacheable(cacheNames = "allCatsByUser", key = "#user.userId")
public List<CatOutputDTO> getAllCatsByUser(User user) {Also, if you ever need to use complex types as keys in the future, ensure they have consistent 📝 Committable suggestion
Suggested change
|
||||||||||||
| return catRepository.findAllByUserCatOwner(user).stream().map(catMapper::toDto).toList(); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| @Cacheable(cacheNames = "allCatsByName", key = "#name") | ||||||||||||
| public List<CatOutputDTO> getCatsByName(String name) { | ||||||||||||
| return catRepository.findByCatNameContainingIgnoreCase(name).stream().map(catMapper::toDto).toList(); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| @Cacheable(cacheNames = "catById", key = "#id") | ||||||||||||
| public CatOutputDTO getCatDtoById(Long id) { | ||||||||||||
| return catMapper.toDto(findCatById(id)); | ||||||||||||
| } | ||||||||||||
|
|
@@ -136,13 +145,15 @@ public Cat addCat(Cat cat) { | |||||||||||
| return saveCat(cat); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| @CachePut(cacheNames = "putCat", key = "#catId") | ||||||||||||
| public CatOutputDTO updateCat(Long catId, CatUpdateDTO dto) { | ||||||||||||
|
Comment on lines
+148
to
149
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cache update strategy needs improvement for putCat operation. The Either evict all entries from all caches (slower) or update the specific entry in the appropriate cache (more efficient): - @CachePut(cacheNames = "putCat", key = "#catId")
+ @CachePut(cacheNames = "catById", key = "#catId")
+ @CacheEvict(cacheNames = {"allCats", "allCatsByUser", "allCatsByName"}, allEntries = true)
public CatOutputDTO updateCat(Long catId, CatUpdateDTO dto) {📝 Committable suggestion
Suggested change
|
||||||||||||
| Cat cat = findCatById(catId); | ||||||||||||
| log.info("Updating cat: {}", cat.getCatName()); | ||||||||||||
| catMapper.updateCatFromDto(dto, cat); | ||||||||||||
| return catMapper.toDto(saveCat(cat)); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| @CachePut(cacheNames = "patchCat", key = "#catId") | ||||||||||||
| public CatOutputDTO partialUpdateCat(Long catId, Map<String, Object> updates) { | ||||||||||||
|
Comment on lines
+156
to
157
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same cache update issue with partialUpdateCat method. Similar to the previous issue, this method also uses a different cache name "patchCat" that doesn't align with other cache names. Update the cache configuration: - @CachePut(cacheNames = "patchCat", key = "#catId")
+ @CachePut(cacheNames = "catById", key = "#catId")
+ @CacheEvict(cacheNames = {"allCats", "allCatsByUser", "allCatsByName"}, allEntries = true)
public CatOutputDTO partialUpdateCat(Long catId, Map<String, Object> updates) {📝 Committable suggestion
Suggested change
|
||||||||||||
| Cat cat = findCatById(catId); | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,3 +9,17 @@ spring.config.import=optional:file:.env[.properties] | |||||||||
| server.servlet.session.persistent=false | ||||||||||
| catapi.key=${CATAPI_KEY} | ||||||||||
|
|
||||||||||
| # Enable caching | ||||||||||
| spring.cache.type=redis | ||||||||||
|
|
||||||||||
| # Toggle Redis | ||||||||||
| spring.redis.enabled=true | ||||||||||
| spring.data.redis.repositories.enabled=false | ||||||||||
|
|
||||||||||
| # Redis connection details | ||||||||||
| spring.data.redis.host=${SPRING_REDIS_HOST:localhost} | ||||||||||
| spring.data.redis.port=${SPRING_REDIS_PORT:6379} | ||||||||||
|
Comment on lines
+20
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use correct Redis connection properties Spring Boot binds -spring.data.redis.host=${SPRING_REDIS_HOST:localhost}
-spring.data.redis.port=${SPRING_REDIS_PORT:6379}
+spring.redis.host=${SPRING_REDIS_HOST:localhost}
+spring.redis.port=${SPRING_REDIS_PORT:6379}📝 Committable suggestion
Suggested change
|
||||||||||
|
|
||||||||||
| spring.datasource.url=jdbc:postgresql://postgres:5432/mydatabase | ||||||||||
| spring.datasource.username=myuser | ||||||||||
| spring.datasource.password=secret | ||||||||||
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.
💡 Verification agent
🧩 Analysis chain
Verify JDK version choice
JDK 24 is very recent (released in March 2024). Ensure this is the intended version for your project and that all your dependencies are compatible with it.
🌐 Web query:
💡 Result:
Recommended Java Version for Spring Boot Applications in 2024
Minimum and Supported Versions
Recommended Practice
Summary Table
*Java 22–24 are supported, but only Java 17 and 21 are LTS releases, which are preferred for stability and long-term maintenance[1][4].
Conclusion
The best practice for Spring Boot applications in 2024 is to use Java 21 LTS for both new projects and upgrades, unless constrained by environment or dependency requirements. Java 17 remains a viable and supported baseline, but Java 21 offers the best combination of stability, support, and modern Java features[1][4][8].
Citations:
Review JDK Version in Dockerfile
Spring Boot 3.4.x supports Java 17–24, but Java 21 LTS is the recommended runtime for stability and long-term support. JDK 24 is non-LTS and may expose you to shorter support windows.
• File: Dockerfile
Line 7:
FROM openjdk:24-jdkSuggestion: use
openjdk:21-jdk(or at least Java 17) unless you explicitly need JDK 24 features and accept its non-LTS lifecycle.