mirror of
https://github.com/booklore-app/booklore.git
synced 2025-12-23 22:28:11 -05:00
refactor(api): minor QOL updates, code modernization (#1802)
* refactor(api): minor QOL updates, code modernization Signed-off-by: Balázs Szücs <bszucs1209@gmail.com> * refactor(oidc): replace regex literals with compiled patterns for improved performance Signed-off-by: Balázs Szücs <bszucs1209@gmail.com> * refactor(shelf): remove redundant icon type initialization method Signed-off-by: Balázs Szücs <bszucs1209@gmail.com> --------- Signed-off-by: Balázs Szücs <bszucs1209@gmail.com>
This commit is contained in:
@@ -46,7 +46,7 @@ public class AdditionalFileController {
|
||||
@PathVariable Long bookId,
|
||||
@RequestParam("file") MultipartFile file,
|
||||
@RequestParam AdditionalFileType additionalFileType,
|
||||
@RequestParam(required = false) String description) throws IOException {
|
||||
@RequestParam(required = false) String description) {
|
||||
AdditionalFile additionalFile = fileUploadService.uploadAdditionalFile(bookId, file, additionalFileType, description);
|
||||
return ResponseEntity.ok(additionalFile);
|
||||
}
|
||||
|
||||
@@ -33,6 +33,7 @@ import java.nio.charset.StandardCharsets;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
import java.util.concurrent.ConcurrentMap;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
/**
|
||||
* Controller for handling OIDC authentication for mobile applications.
|
||||
@@ -53,6 +54,7 @@ import java.util.concurrent.ConcurrentMap;
|
||||
@RequestMapping("/api/v1/auth/mobile")
|
||||
public class MobileOidcController {
|
||||
|
||||
private static final Pattern TRAILING_SLASHES_PATTERN = Pattern.compile("/+$");
|
||||
private final AppSettingService appSettingService;
|
||||
private final UserRepository userRepository;
|
||||
private final UserProvisioningService userProvisioningService;
|
||||
@@ -213,7 +215,7 @@ public class MobileOidcController {
|
||||
* Discover the token endpoint from the OIDC provider's well-known configuration.
|
||||
*/
|
||||
private String discoverTokenEndpoint(String issuerUri) throws Exception {
|
||||
String discoveryUrl = issuerUri.replaceAll("/+$", "") + "/.well-known/openid-configuration";
|
||||
String discoveryUrl = TRAILING_SLASHES_PATTERN.matcher(issuerUri).replaceAll("") + "/.well-known/openid-configuration";
|
||||
|
||||
RestTemplate restTemplate = new RestTemplate();
|
||||
ResponseEntity<String> response = restTemplate.getForEntity(discoveryUrl, String.class);
|
||||
@@ -227,7 +229,7 @@ public class MobileOidcController {
|
||||
|
||||
if (tokenEndpointNode == null || tokenEndpointNode.isNull()) {
|
||||
// Fall back to standard path
|
||||
return issuerUri.replaceAll("/+$", "") + "/protocol/openid-connect/token";
|
||||
return TRAILING_SLASHES_PATTERN.matcher(issuerUri).replaceAll("") + "/protocol/openid-connect/token";
|
||||
}
|
||||
|
||||
return tokenEndpointNode.asText();
|
||||
@@ -241,7 +243,7 @@ public class MobileOidcController {
|
||||
String code,
|
||||
String codeVerifier,
|
||||
String redirectUri,
|
||||
String clientId) throws Exception {
|
||||
String clientId) {
|
||||
|
||||
RestTemplate restTemplate = new RestTemplate();
|
||||
|
||||
|
||||
@@ -29,7 +29,7 @@ public class AuthorEntity {
|
||||
public boolean equals(Object o) {
|
||||
if (this == o) return true;
|
||||
if (!(o instanceof AuthorEntity that)) return false;
|
||||
return getId() != null && Objects.equals(getId(), that.getId());
|
||||
return id != null && Objects.equals(id, that.id);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@@ -31,7 +31,7 @@ public class CategoryEntity {
|
||||
public boolean equals(Object o) {
|
||||
if (this == o) return true;
|
||||
if (!(o instanceof CategoryEntity that)) return false;
|
||||
return getId() != null && Objects.equals(getId(), that.getId());
|
||||
return id != null && Objects.equals(id, that.id);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@@ -43,7 +43,8 @@ public class LibraryEntity {
|
||||
|
||||
@Enumerated(EnumType.STRING)
|
||||
@Column(name = "icon_type", nullable = false)
|
||||
private IconType iconType;
|
||||
@Builder.Default
|
||||
private IconType iconType = IconType.PRIME_NG;
|
||||
|
||||
@Column(name = "file_naming_pattern")
|
||||
private String fileNamingPattern;
|
||||
@@ -56,4 +57,11 @@ public class LibraryEntity {
|
||||
@Enumerated(EnumType.STRING)
|
||||
@Column(name = "default_book_format")
|
||||
private BookFileType defaultBookFormat;
|
||||
|
||||
@PrePersist
|
||||
public void ensureIconType() {
|
||||
if (this.iconType == null) {
|
||||
this.iconType = IconType.PRIME_NG;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -32,7 +32,9 @@ public class MagicShelfEntity {
|
||||
|
||||
@Enumerated(EnumType.STRING)
|
||||
@Column(name = "icon_type", nullable = false)
|
||||
private IconType iconType;
|
||||
|
||||
@Builder.Default
|
||||
private IconType iconType = IconType.PRIME_NG;
|
||||
|
||||
@Column(name = "filter_json", columnDefinition = "json", nullable = false)
|
||||
private String filterJson;
|
||||
@@ -53,4 +55,11 @@ public class MagicShelfEntity {
|
||||
public void onUpdate() {
|
||||
updatedAt = LocalDateTime.now();
|
||||
}
|
||||
|
||||
@PrePersist
|
||||
public void ensureIconType() {
|
||||
if (this.iconType == null) {
|
||||
this.iconType = IconType.PRIME_NG;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -31,7 +31,7 @@ public class MoodEntity {
|
||||
public boolean equals(Object o) {
|
||||
if (this == o) return true;
|
||||
if (!(o instanceof MoodEntity that)) return false;
|
||||
return getId() != null && Objects.equals(getId(), that.getId());
|
||||
return id != null && Objects.equals(id, that.id);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@@ -37,7 +37,8 @@ public class ShelfEntity {
|
||||
|
||||
@Enumerated(EnumType.STRING)
|
||||
@Column(name = "icon_type", nullable = false)
|
||||
private IconType iconType;
|
||||
@Builder.Default
|
||||
private IconType iconType = IconType.PRIME_NG;
|
||||
|
||||
@ManyToMany(fetch = FetchType.LAZY)
|
||||
@JoinTable(
|
||||
|
||||
@@ -31,7 +31,7 @@ public class TagEntity {
|
||||
public boolean equals(Object o) {
|
||||
if (this == o) return true;
|
||||
if (!(o instanceof TagEntity that)) return false;
|
||||
return getId() != null && Objects.equals(getId(), that.getId());
|
||||
return id != null && Objects.equals(id, that.id);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@@ -17,10 +17,7 @@ import java.time.LocalDate;
|
||||
import java.time.LocalDateTime;
|
||||
import java.time.ZoneId;
|
||||
import java.time.format.DateTimeFormatter;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.*;
|
||||
import java.util.function.BiFunction;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
@@ -433,7 +430,7 @@ public class BookRuleEvaluatorService {
|
||||
private List<String> toStringList(Object value) {
|
||||
if (value == null) return Collections.emptyList();
|
||||
if (value instanceof List) {
|
||||
return ((List<?>) value).stream()
|
||||
return ((Collection<?>) value).stream()
|
||||
.map(Object::toString)
|
||||
.collect(Collectors.toList());
|
||||
}
|
||||
|
||||
@@ -21,6 +21,7 @@ import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
import java.util.regex.Pattern;
|
||||
import java.util.stream.Stream;
|
||||
|
||||
@Slf4j
|
||||
@@ -28,6 +29,7 @@ import java.util.stream.Stream;
|
||||
@Service
|
||||
public class IconService {
|
||||
|
||||
private static final Pattern INVALID_FILENAME_CHARS_PATTERN = Pattern.compile("[^a-zA-Z0-9._-]");
|
||||
private final AppProperties appProperties;
|
||||
|
||||
private final ConcurrentHashMap<String, String> svgCache = new ConcurrentHashMap<>();
|
||||
@@ -262,7 +264,7 @@ public class IconService {
|
||||
throw ApiError.INVALID_INPUT.createException("Filename cannot be empty");
|
||||
}
|
||||
|
||||
String sanitized = filename.trim().replaceAll("[^a-zA-Z0-9._-]", "_");
|
||||
String sanitized = INVALID_FILENAME_CHARS_PATTERN.matcher(filename.trim()).replaceAll("_");
|
||||
return sanitized.endsWith(SVG_EXTENSION) ? sanitized : sanitized + SVG_EXTENSION;
|
||||
}
|
||||
|
||||
|
||||
@@ -291,11 +291,9 @@ public class CbxConversionService {
|
||||
private boolean isImageFile(String fileName) {
|
||||
String lowerName = fileName.toLowerCase();
|
||||
|
||||
boolean isImage = lowerName.endsWith(".jpg") || lowerName.endsWith(".jpeg") ||
|
||||
return lowerName.endsWith(".jpg") || lowerName.endsWith(".jpeg") ||
|
||||
lowerName.endsWith(".png") || lowerName.endsWith(".webp") ||
|
||||
lowerName.endsWith(".gif") || lowerName.endsWith(".bmp");
|
||||
|
||||
return isImage;
|
||||
}
|
||||
|
||||
private boolean isJpegFile(Path path) {
|
||||
@@ -348,7 +346,7 @@ public class CbxConversionService {
|
||||
List<EpubContentFileGroup> contentGroups = new ArrayList<>();
|
||||
|
||||
if (!imagePaths.isEmpty()) {
|
||||
addImageToZipFromPath(zipOut, COVER_IMAGE_PATH, imagePaths.get(0));
|
||||
addImageToZipFromPath(zipOut, COVER_IMAGE_PATH, imagePaths.getFirst());
|
||||
}
|
||||
|
||||
for (int i = 0; i < imagePaths.size(); i++) {
|
||||
@@ -461,7 +459,7 @@ public class CbxConversionService {
|
||||
model.put("tocNcxPath", makeRelativeToOebps(TOC_NCX_PATH));
|
||||
model.put("navXhtmlPath", makeRelativeToOebps(NAV_XHTML_PATH));
|
||||
model.put("stylesheetCssPath", makeRelativeToOebps(STYLESHEET_CSS_PATH));
|
||||
model.put("firstPageId", contentGroups.isEmpty() ? "" : "page_" + contentGroups.get(0).contentKey());
|
||||
model.put("firstPageId", contentGroups.isEmpty() ? "" : "page_" + contentGroups.getFirst().contentKey());
|
||||
|
||||
String contentOpf = processTemplate("xml/content.opf.ftl", model);
|
||||
|
||||
|
||||
@@ -201,8 +201,7 @@ public class KoboReadingStateService {
|
||||
return true;
|
||||
}
|
||||
|
||||
boolean withinSyncBufferWindow = now.isBefore(statusSentTime.plusSeconds(STATUS_SYNC_BUFFER_SECONDS));
|
||||
return withinSyncBufferWindow;
|
||||
return now.isBefore(statusSentTime.plusSeconds(STATUS_SYNC_BUFFER_SECONDS));
|
||||
}
|
||||
|
||||
private ReadStatus deriveStatusFromProgress(double progressPercent) {
|
||||
|
||||
@@ -64,15 +64,14 @@ public class DuckDuckGoCoverService implements BookCoverProvider {
|
||||
siteFilteredImages = siteFilteredImages.subList(0, 7);
|
||||
}
|
||||
|
||||
String generalQuery = searchTerm;
|
||||
String encodedGeneralQuery = URLEncoder.encode(generalQuery, StandardCharsets.UTF_8);
|
||||
String encodedGeneralQuery = URLEncoder.encode(searchTerm, StandardCharsets.UTF_8);
|
||||
String generalUrl = SEARCH_BASE_URL + encodedGeneralQuery + SEARCH_PARAMS;
|
||||
Document generalDoc = getDocument(generalUrl);
|
||||
Matcher generalMatcher = tokenPattern.matcher(generalDoc.html());
|
||||
List<CoverImage> generalBookImages = new ArrayList<>();
|
||||
if (generalMatcher.find()) {
|
||||
String generalSearchToken = generalMatcher.group(1);
|
||||
generalBookImages = fetchImagesFromApi(generalQuery, generalSearchToken);
|
||||
generalBookImages = fetchImagesFromApi(searchTerm, generalSearchToken);
|
||||
generalBookImages.removeIf(dto -> dto.getWidth() < 350);
|
||||
generalBookImages.removeIf(dto -> dto.getWidth() >= dto.getHeight());
|
||||
Set<String> siteUrls = siteFilteredImages.stream().map(CoverImage::getUrl).collect(Collectors.toSet());
|
||||
|
||||
@@ -361,9 +361,9 @@ public class EpubMetadataExtractor implements FileMetadataExtractor {
|
||||
// Normalize path components to handle ".." and "."
|
||||
java.util.LinkedList<String> parts = new java.util.LinkedList<>();
|
||||
for (String part : combined.split("/")) {
|
||||
if (part.equals("..")) {
|
||||
if ("..".equals(part)) {
|
||||
if (!parts.isEmpty()) parts.removeLast();
|
||||
} else if (!part.equals(".") && !part.isEmpty()) {
|
||||
} else if (!".".equals(part) && !part.isEmpty()) {
|
||||
parts.add(part);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -198,51 +198,47 @@ public class CbxMetadataWriter implements MetadataWriter {
|
||||
|
||||
if (rarAvailable) {
|
||||
tempDir = Files.createTempDirectory("cbx_rar_");
|
||||
try {
|
||||
// Extract entire RAR into a temp directory
|
||||
try (Archive archive = new Archive(file)) {
|
||||
for (FileHeader fh : archive.getFileHeaders()) {
|
||||
String name = fh.getFileName();
|
||||
if (name == null || name.isBlank()) continue;
|
||||
if (!isSafeEntryName(name)) {
|
||||
log.warn("Skipping unsafe RAR entry name: {}", name);
|
||||
continue;
|
||||
}
|
||||
Path out = tempDir.resolve(name).normalize();
|
||||
if (!out.startsWith(tempDir)) {
|
||||
log.warn("Skipping traversal entry outside tempDir: {}", name);
|
||||
continue;
|
||||
}
|
||||
if (fh.isDirectory()) {
|
||||
Files.createDirectories(out);
|
||||
} else {
|
||||
Files.createDirectories(out.getParent());
|
||||
try (OutputStream os = Files.newOutputStream(out, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) {
|
||||
archive.extractFile(fh, os);
|
||||
}
|
||||
// Extract entire RAR into a temp directory
|
||||
try (Archive archive = new Archive(file)) {
|
||||
for (FileHeader fh : archive.getFileHeaders()) {
|
||||
String name = fh.getFileName();
|
||||
if (name == null || name.isBlank()) continue;
|
||||
if (!isSafeEntryName(name)) {
|
||||
log.warn("Skipping unsafe RAR entry name: {}", name);
|
||||
continue;
|
||||
}
|
||||
Path out = tempDir.resolve(name).normalize();
|
||||
if (!out.startsWith(tempDir)) {
|
||||
log.warn("Skipping traversal entry outside tempDir: {}", name);
|
||||
continue;
|
||||
}
|
||||
if (fh.isDirectory()) {
|
||||
Files.createDirectories(out);
|
||||
} else {
|
||||
Files.createDirectories(out.getParent());
|
||||
try (OutputStream os = Files.newOutputStream(out, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) {
|
||||
archive.extractFile(fh, os);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Write/replace ComicInfo.xml in extracted tree root
|
||||
Path comicInfo = tempDir.resolve("ComicInfo.xml");
|
||||
Files.write(comicInfo, xmlBytes);
|
||||
// Write/replace ComicInfo.xml in extracted tree root
|
||||
Path comicInfo = tempDir.resolve("ComicInfo.xml");
|
||||
Files.write(comicInfo, xmlBytes);
|
||||
|
||||
// Rebuild RAR in-place (replace original file)
|
||||
Path targetRar = file.toPath().toAbsolutePath().normalize();
|
||||
String rarExec = isSafeExecutable(rarBin) ? rarBin : "rar"; // prefer validated path, then PATH lookup
|
||||
ProcessBuilder pb = new ProcessBuilder(rarExec, "a", "-idq", "-ep1", "-ma5", targetRar.toString(), ".");
|
||||
pb.directory(tempDir.toFile());
|
||||
Process p = pb.start();
|
||||
int code = p.waitFor();
|
||||
if (code == 0) {
|
||||
writeSucceeded = true;
|
||||
return;
|
||||
} else {
|
||||
log.warn("RAR creation failed with exit code {}. Falling back to CBZ conversion for {}", code, file.getName());
|
||||
}
|
||||
} finally {
|
||||
// tempDir cleanup will be handled in outer finally block
|
||||
// Rebuild RAR in-place (replace original file)
|
||||
Path targetRar = file.toPath().toAbsolutePath().normalize();
|
||||
String rarExec = isSafeExecutable(rarBin) ? rarBin : "rar"; // prefer validated path, then PATH lookup
|
||||
ProcessBuilder pb = new ProcessBuilder(rarExec, "a", "-idq", "-ep1", "-ma5", targetRar.toString(), ".");
|
||||
pb.directory(tempDir.toFile());
|
||||
Process p = pb.start();
|
||||
int code = p.waitFor();
|
||||
if (code == 0) {
|
||||
writeSucceeded = true;
|
||||
return;
|
||||
} else {
|
||||
log.warn("RAR creation failed with exit code {}. Falling back to CBZ conversion for {}", code, file.getName());
|
||||
}
|
||||
} else {
|
||||
log.warn("`rar` binary not found. Falling back to CBZ conversion for {}", file.getName());
|
||||
|
||||
@@ -82,7 +82,7 @@ class KoboEntitlementServiceTest {
|
||||
|
||||
assertNotNull(result);
|
||||
assertEquals(1, result.getDownloadUrls().size());
|
||||
assertEquals(KoboBookFormat.EPUB3.toString(), result.getDownloadUrls().get(0).getFormat());
|
||||
assertEquals(KoboBookFormat.EPUB3.toString(), result.getDownloadUrls().getFirst().getFormat());
|
||||
}
|
||||
|
||||
private BookEntity createCbxBookEntity(Long id) {
|
||||
|
||||
@@ -83,7 +83,7 @@ class FileMoveServiceTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void moveSingleFile_whenLibraryMonitored_reRegistersLibraryPaths() throws Exception {
|
||||
void moveSingleFile_whenLibraryMonitored_reRegistersLibraryPaths() {
|
||||
when(monitoringRegistrationService.isLibraryMonitored(42L)).thenReturn(true);
|
||||
|
||||
FileMoveResult result = fileMoveService.moveSingleFile(bookEntity);
|
||||
@@ -96,7 +96,7 @@ class FileMoveServiceTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void moveSingleFile_whenLibraryNotMonitored_skipsMonitoringCalls() throws Exception {
|
||||
void moveSingleFile_whenLibraryNotMonitored_skipsMonitoringCalls() {
|
||||
when(monitoringRegistrationService.isLibraryMonitored(42L)).thenReturn(false);
|
||||
|
||||
FileMoveResult result = fileMoveService.moveSingleFile(bookEntity);
|
||||
|
||||
@@ -1,10 +1,15 @@
|
||||
package com.adityachandel.booklore.util;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.*;
|
||||
|
||||
class Md5UtilTest {
|
||||
|
||||
private static final Pattern PATTERN = Pattern.compile("[a-f0-9]{32}");
|
||||
|
||||
@Test
|
||||
void testMd5Hex_emptyString() {
|
||||
String result = Md5Util.md5Hex("");
|
||||
@@ -75,6 +80,6 @@ class Md5UtilTest {
|
||||
void testMd5Hex_length() {
|
||||
String result = Md5Util.md5Hex("any input");
|
||||
assertEquals(32, result.length()); // MD5 always produces 32 character hex string
|
||||
assertTrue(result.matches("[a-f0-9]{32}")); // Only lowercase hex characters
|
||||
assertTrue(PATTERN.matcher(result).matches()); // Only lowercase hex characters
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user