Junior

package com.example.photogallery.controller; import com.example.photogallery.model.Photo; import com.example.photogallery.service.PhotoService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.web.bind.annotation.*; import org.springframework.web.multipart.MultipartFile; import java.util.ArrayList; import java.util.List; @RestController @RequestMapping("/api/photos") public class PhotoController { @Autowired private PhotoService photoService; private List<Photo> photos = new ArrayList<>(); private static Integer totalCount = 0; @GetMapping public List<Photo> getAllPhotos() { return photos; } @PostMapping("/upload") public String uploadPhoto(@RequestParam("file") MultipartFile file) { try { Photo photo = new Photo(); photo.setFileName(file.getOriginalFilename()); photo.setFileSize(file.getSize()); photo.setContentType(file.getContentType()); photos.add(photo); totalCount++; photoService.savePhoto(photo); return "Photo uploaded successfully"; } catch (Exception e) { return "Error uploading photo"; } } @GetMapping("/count") public Integer getCount() { return totalCount; } @DeleteMapping("/{id}") public void deletePhoto(@PathVariable Long id) { for (Photo photo : photos) { if (photo.getId().equals(id)) { photos.remove(photo); totalCount--; photoService.deletePhoto(id); break; } } } @GetMapping("/search") public List<Photo> searchPhotos(@RequestParam(required = false) String query) { if (query.isEmpty()) { return photos; } List<Photo> results = new ArrayList<>(); for (int i = 0; i < photos.size(); i++) { if (photos.get(i).getFileName().contains(query)) { results.add(photos.get(i)); } } return results; } }

sobes.tech AI

Answer from AI

The presented controller has several issues:

Storing data in the controller's memory: the list photos and the counter totalCount are in the controller, which is not suitable for a multithreaded environment and does not persist between application restarts.
Potential problems with removing items from the list during iteration: removing an element from the list inside a for loop over the same list can lead to a ConcurrentModificationException.
Checking query.isEmpty() without null check: can cause a NullPointerException.
Lack of error handling and HTTP status codes: methods return strings or void; it's better to use ResponseEntity with appropriate statuses.
Lack of synchronization: concurrent requests may cause data consistency issues.

Recommendations:

Move photo storage to a service or database.
Use thread-safe collections or synchronization.
Use an iterator or filtering in the delete method.
Add null checks for parameters.
Return proper HTTP responses.

Example of fixing the delete method:

@DeleteMapping("/{id}")
public ResponseEntity<Void> deletePhoto(@PathVariable Long id) {
    boolean removed = photos.removeIf(photo -> photo.getId().equals(id));
    if (removed) {
        totalCount--;
        photoService.deletePhoto(id);
        return ResponseEntity.ok().build();
    } else {
        return ResponseEntity.notFound().build();
    }
}

It's better to store data in a database via a service rather than in the controller.

Previous Next