Skip to content

Commit 90e4afd

Browse files
davidemeclaude
andcommitted
feat(java): improve Java 21 idiomatics
- Replace .collect(Collectors.toList()) with .toList() (Java 16+) - Use pattern matching for instanceof in LampEntity.equals() (Java 16+) - Add LampNotFoundException for explicit 404 handling via @ControllerAdvice - Change LampService.findById() to return Optional<Lamp> - Change LampService.update()/delete() to throw LampNotFoundException - Remove @SuppressWarnings("unchecked") and unsafe double-cast patterns - Remove try/catch blocks from controller (GlobalExceptionHandler handles them) Closes #352 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 991da10 commit 90e4afd

9 files changed

Lines changed: 121 additions & 94 deletions

File tree

src/java/src/main/java/org/openapitools/controller/LampsController.java

Lines changed: 13 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import java.util.concurrent.CompletableFuture;
77
import lombok.RequiredArgsConstructor;
88
import org.openapitools.api.LampsApi;
9-
import org.openapitools.model.Error;
9+
import org.openapitools.exception.LampNotFoundException;
1010
import org.openapitools.model.Lamp;
1111
import org.openapitools.model.LampCreate;
1212
import org.openapitools.model.LampUpdate;
@@ -36,44 +36,23 @@ public CompletableFuture<ResponseEntity<Lamp>> createLamp(final LampCreate lampC
3636
}
3737

3838
@Override
39-
@SuppressWarnings("unchecked")
4039
public CompletableFuture<ResponseEntity<Void>> deleteLamp(final String lampId) {
4140
return CompletableFuture.supplyAsync(
4241
() -> {
43-
try {
44-
final UUID lampUuid = UUID.fromString(lampId);
45-
final boolean deleted = lampService.delete(lampUuid);
46-
if (deleted) {
47-
return ResponseEntity.noContent().<Void>build();
48-
} else {
49-
return ResponseEntity.notFound().<Void>build();
50-
}
51-
} catch (IllegalArgumentException e) {
52-
final Error error = new Error("INVALID_ARGUMENT");
53-
return (ResponseEntity<Void>)
54-
(ResponseEntity<?>) ResponseEntity.badRequest().body(error);
55-
}
42+
final UUID lampUuid = UUID.fromString(lampId);
43+
lampService.delete(lampUuid);
44+
return ResponseEntity.noContent().<Void>build();
5645
});
5746
}
5847

5948
@Override
60-
@SuppressWarnings("unchecked")
6149
public CompletableFuture<ResponseEntity<Lamp>> getLamp(final String lampId) {
6250
return CompletableFuture.supplyAsync(
6351
() -> {
64-
try {
65-
final UUID lampUuid = UUID.fromString(lampId);
66-
final Lamp lamp = lampService.findById(lampUuid);
67-
if (lamp != null) {
68-
return ResponseEntity.ok().body(lamp);
69-
} else {
70-
return ResponseEntity.notFound().build();
71-
}
72-
} catch (IllegalArgumentException e) {
73-
final Error error = new Error("INVALID_ARGUMENT");
74-
return (ResponseEntity<Lamp>)
75-
(ResponseEntity<?>) ResponseEntity.badRequest().body(error);
76-
}
52+
final UUID lampUuid = UUID.fromString(lampId);
53+
final Lamp lamp =
54+
lampService.findById(lampUuid).orElseThrow(() -> new LampNotFoundException(lampUuid));
55+
return ResponseEntity.ok().body(lamp);
7756
});
7857
}
7958

@@ -91,26 +70,15 @@ public CompletableFuture<ResponseEntity<ListLamps200Response>> listLamps(
9170
}
9271

9372
@Override
94-
@SuppressWarnings("unchecked")
9573
public CompletableFuture<ResponseEntity<Lamp>> updateLamp(
9674
final String lampId, final LampUpdate lampUpdate) {
9775
return CompletableFuture.supplyAsync(
9876
() -> {
99-
try {
100-
final UUID lampUuid = UUID.fromString(lampId);
101-
final Lamp lampData = new Lamp();
102-
lampData.setStatus(lampUpdate.getStatus());
103-
final Lamp updated = lampService.update(lampUuid, lampData);
104-
if (updated != null) {
105-
return ResponseEntity.ok().body(updated);
106-
} else {
107-
return ResponseEntity.notFound().build();
108-
}
109-
} catch (IllegalArgumentException e) {
110-
final Error error = new Error("INVALID_ARGUMENT");
111-
return (ResponseEntity<Lamp>)
112-
(ResponseEntity<?>) ResponseEntity.badRequest().body(error);
113-
}
77+
final UUID lampUuid = UUID.fromString(lampId);
78+
final Lamp lampData = new Lamp();
79+
lampData.setStatus(lampUpdate.getStatus());
80+
final Lamp updated = lampService.update(lampUuid, lampData);
81+
return ResponseEntity.ok().body(updated);
11482
});
11583
}
11684
}

src/java/src/main/java/org/openapitools/exception/GlobalExceptionHandler.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,17 @@ public ResponseEntity<Error> handleNullPointerException(final NullPointerExcepti
7070
return new ResponseEntity<>(error, HttpStatus.BAD_REQUEST);
7171
}
7272

73+
/**
74+
* Handle lamp not found exceptions.
75+
*
76+
* @param ex the lamp not found exception
77+
* @return 404 Not Found
78+
*/
79+
@ExceptionHandler(LampNotFoundException.class)
80+
public ResponseEntity<Void> handleLampNotFoundException(final LampNotFoundException ex) {
81+
return ResponseEntity.notFound().build();
82+
}
83+
7384
/**
7485
* Handle illegal argument exceptions (e.g., invalid UUID format).
7586
*
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package org.openapitools.exception;
2+
3+
import java.util.UUID;
4+
5+
/**
6+
* Exception thrown when a lamp with a given ID cannot be found. This is a domain-level exception
7+
* that maps to HTTP 404 responses.
8+
*/
9+
public class LampNotFoundException extends RuntimeException {
10+
11+
private final UUID lampId;
12+
13+
public LampNotFoundException(final UUID lampId) {
14+
super("Lamp not found: " + lampId);
15+
this.lampId = lampId;
16+
}
17+
18+
public UUID getLampId() {
19+
return lampId;
20+
}
21+
}

src/java/src/main/java/org/openapitools/repository/impl/InMemoryLampRepository.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import java.util.Optional;
88
import java.util.UUID;
99
import java.util.concurrent.ConcurrentHashMap;
10-
import java.util.stream.Collectors;
1110
import org.openapitools.config.OnNoDatabaseUrlCondition;
1211
import org.openapitools.entity.LampEntity;
1312
import org.openapitools.repository.LampRepository;
@@ -129,15 +128,15 @@ public List<LampEntity> findByStatus(final Boolean isOn) {
129128
return lamps.values().stream()
130129
.filter(lamp -> lamp.getDeletedAt() == null)
131130
.filter(lamp -> lamp.getStatus().equals(isOn))
132-
.collect(Collectors.toList());
131+
.toList();
133132
}
134133

135134
@Override
136135
public List<LampEntity> findAllActive() {
137136
return lamps.values().stream()
138137
.filter(lamp -> lamp.getDeletedAt() == null)
139138
.sorted(Comparator.comparing(LampEntity::getCreatedAt))
140-
.collect(Collectors.toList());
139+
.toList();
141140
}
142141

143142
@Override

src/java/src/main/java/org/openapitools/service/LampService.java

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22

33
import java.time.OffsetDateTime;
44
import java.util.List;
5+
import java.util.Optional;
56
import java.util.UUID;
6-
import java.util.stream.Collectors;
77
import lombok.RequiredArgsConstructor;
88
import org.openapitools.entity.LampEntity;
9+
import org.openapitools.exception.LampNotFoundException;
910
import org.openapitools.mapper.LampMapper;
1011
import org.openapitools.model.Lamp;
1112
import org.openapitools.repository.LampRepository;
@@ -57,10 +58,10 @@ public Lamp create(final Lamp lamp) {
5758
* Find a lamp by its ID.
5859
*
5960
* @param id the lamp ID
60-
* @return the lamp if found, null otherwise
61+
* @return optional containing the lamp if found, empty otherwise
6162
*/
62-
public Lamp findById(final UUID id) {
63-
return repository.findById(id).map(mapper::toModel).orElse(null);
63+
public Optional<Lamp> findById(final UUID id) {
64+
return repository.findById(id).map(mapper::toModel);
6465
}
6566

6667
/**
@@ -75,7 +76,7 @@ public List<Lamp> findAll(final int offset, final int limit) {
7576
PageRequest.of(offset / limit, limit, Sort.by(Sort.Direction.ASC, "createdAt"));
7677

7778
final Page<LampEntity> page = repository.findAll(pageable);
78-
return page.getContent().stream().map(mapper::toModel).collect(Collectors.toList());
79+
return page.getContent().stream().map(mapper::toModel).toList();
7980
}
8081

8182
/**
@@ -84,7 +85,7 @@ public List<Lamp> findAll(final int offset, final int limit) {
8485
* @return list of all active lamps ordered by creation time
8586
*/
8687
public List<Lamp> findAllActive() {
87-
return repository.findAllActive().stream().map(mapper::toModel).collect(Collectors.toList());
88+
return repository.findAllActive().stream().map(mapper::toModel).toList();
8889
}
8990

9091
/**
@@ -94,7 +95,7 @@ public List<Lamp> findAllActive() {
9495
* @return list of lamps with the specified status
9596
*/
9697
public List<Lamp> findByStatus(final Boolean isOn) {
97-
return repository.findByStatus(isOn).stream().map(mapper::toModel).collect(Collectors.toList());
98+
return repository.findByStatus(isOn).stream().map(mapper::toModel).toList();
9899
}
99100

100101
/**
@@ -111,7 +112,8 @@ public long countActive() {
111112
*
112113
* @param id the lamp ID
113114
* @param lamp the updated lamp data
114-
* @return the updated lamp if found, null otherwise
115+
* @return the updated lamp
116+
* @throws LampNotFoundException if no lamp exists with the given ID
115117
*/
116118
@Transactional
117119
public Lamp update(final UUID id, final Lamp lamp) {
@@ -123,7 +125,7 @@ public Lamp update(final UUID id, final Lamp lamp) {
123125
// updatedAt is automatically set by @UpdateTimestamp
124126
return mapper.toModel(repository.save(entity));
125127
})
126-
.orElse(null);
128+
.orElseThrow(() -> new LampNotFoundException(id));
127129
}
128130

129131
/**
@@ -133,18 +135,13 @@ public Lamp update(final UUID id, final Lamp lamp) {
133135
* LampEntity.
134136
*
135137
* @param id the lamp ID to delete
136-
* @return true if the lamp was found and deleted, false otherwise
138+
* @throws LampNotFoundException if no lamp exists with the given ID
137139
*/
138140
@Transactional
139-
public boolean delete(final UUID id) {
140-
return repository
141-
.findById(id)
142-
.map(
143-
entity -> {
144-
entity.setDeletedAt(OffsetDateTime.now());
145-
repository.save(entity);
146-
return true;
147-
})
148-
.orElse(false);
141+
public void delete(final UUID id) {
142+
final LampEntity entity =
143+
repository.findById(id).orElseThrow(() -> new LampNotFoundException(id));
144+
entity.setDeletedAt(OffsetDateTime.now());
145+
repository.save(entity);
149146
}
150147
}

src/java/src/test/java/org/openapitools/controller/LampsControllerTest.java

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
package org.openapitools.controller;
22

33
import static org.mockito.ArgumentMatchers.any;
4+
import static org.mockito.Mockito.doThrow;
45
import static org.mockito.Mockito.when;
56
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
67
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
78

89
import com.fasterxml.jackson.databind.ObjectMapper;
910
import java.util.Arrays;
1011
import java.util.List;
12+
import java.util.Optional;
1113
import java.util.UUID;
1214
import org.junit.jupiter.api.BeforeEach;
1315
import org.junit.jupiter.api.Test;
16+
import org.openapitools.exception.LampNotFoundException;
1417
import org.openapitools.model.Lamp;
1518
import org.openapitools.model.LampCreate;
1619
import org.openapitools.model.LampUpdate;
@@ -65,7 +68,7 @@ void listLamps_ShouldReturnAllLamps() throws Exception {
6568
@Test
6669
void getLamp_WithValidId_ShouldReturnLamp() throws Exception {
6770
// Given
68-
when(lampService.findById(testLampId)).thenReturn(testLamp);
71+
when(lampService.findById(testLampId)).thenReturn(Optional.of(testLamp));
6972

7073
// When & Then
7174
MvcResult result =
@@ -85,7 +88,7 @@ void getLamp_WithValidId_ShouldReturnLamp() throws Exception {
8588
@Test
8689
void getLamp_WithNonExistentId_ShouldReturn404() throws Exception {
8790
// Given
88-
when(lampService.findById(testLampId)).thenReturn(null);
91+
when(lampService.findById(testLampId)).thenReturn(Optional.empty());
8992

9093
// When & Then
9194
MvcResult result =
@@ -172,7 +175,8 @@ void updateLamp_WithNonExistentId_ShouldReturn404() throws Exception {
172175
LampUpdate lampUpdate = new LampUpdate();
173176
lampUpdate.setStatus(false);
174177

175-
when(lampService.update(any(UUID.class), any(Lamp.class))).thenReturn(null);
178+
when(lampService.update(any(UUID.class), any(Lamp.class)))
179+
.thenThrow(new LampNotFoundException(testLampId));
176180

177181
// When & Then
178182
MvcResult result =
@@ -190,8 +194,7 @@ void updateLamp_WithNonExistentId_ShouldReturn404() throws Exception {
190194

191195
@Test
192196
void deleteLamp_WithValidId_ShouldDelete() throws Exception {
193-
// Given
194-
when(lampService.delete(testLampId)).thenReturn(true);
197+
// Given — delete is void, no mock setup needed
195198

196199
// When & Then
197200
MvcResult result =
@@ -206,7 +209,7 @@ void deleteLamp_WithValidId_ShouldDelete() throws Exception {
206209
@Test
207210
void deleteLamp_WithNonExistentId_ShouldReturn404() throws Exception {
208211
// Given
209-
when(lampService.delete(testLampId)).thenReturn(false);
212+
doThrow(new LampNotFoundException(testLampId)).when(lampService).delete(testLampId);
210213

211214
// When & Then
212215
MvcResult result =

src/java/src/test/java/org/openapitools/exception/GlobalExceptionHandlerUnitTest.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static org.assertj.core.api.Assertions.assertThat;
44

5+
import java.util.UUID;
56
import org.junit.jupiter.api.Test;
67
import org.openapitools.model.Error;
78
import org.springframework.http.HttpStatus;
@@ -23,6 +24,16 @@ void handleNullPointerException_ShouldReturnBadRequest() {
2324
assertThat(response.getBody().getError()).isEqualTo("INVALID_ARGUMENT");
2425
}
2526

27+
@Test
28+
void handleLampNotFoundException_ShouldReturnNotFound() {
29+
LampNotFoundException ex = new LampNotFoundException(UUID.randomUUID());
30+
31+
ResponseEntity<Void> response = handler.handleLampNotFoundException(ex);
32+
33+
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
34+
assertThat(response.getBody()).isNull();
35+
}
36+
2637
@Test
2738
void handleIllegalArgumentException_ShouldReturnBadRequest() {
2839
IllegalArgumentException ex = new IllegalArgumentException("bad argument");
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package org.openapitools.exception;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import java.util.UUID;
6+
import org.junit.jupiter.api.Test;
7+
8+
/** Unit tests for LampNotFoundException. */
9+
class LampNotFoundExceptionTest {
10+
11+
@Test
12+
void shouldContainLampIdInMessage() {
13+
final UUID lampId = UUID.randomUUID();
14+
15+
final LampNotFoundException ex = new LampNotFoundException(lampId);
16+
17+
assertThat(ex.getMessage()).contains(lampId.toString());
18+
assertThat(ex.getLampId()).isEqualTo(lampId);
19+
}
20+
}

0 commit comments

Comments
 (0)