feat(api): allow disable of kobo store proxy#337
Merged
zachyale merged 10 commits intoApr 7, 2026
Merged
Conversation
f55431d to
b41928c
Compare
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a boolean app setting Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant KoboController
participant AppSettingService
participant KoboInitializationService
participant KoboServerProxy
participant LocalResources
Client->>KoboController: Request (device token, path)
KoboController->>AppSettingService: getAppSettings().getKoboSettings().isForwardToKoboStore()
AppSettingService-->>KoboController: true / false
alt Forwarding Enabled
KoboController->>KoboInitializationService: handle request (token)
KoboInitializationService->>KoboServerProxy: proxyCurrentRequest(token)
KoboServerProxy-->>KoboInitializationService: proxied response
KoboInitializationService->>KoboController: enriched payload
KoboController-->>Client: 200 + proxied payload
else Forwarding Disabled
KoboController->>KoboInitializationService: handle request (token)
KoboInitializationService->>LocalResources: getResources()
LocalResources-->>KoboInitializationService: local ObjectNode
KoboInitializationService->>KoboController: local payload or empty/404
KoboController-->>Client: 200 (local/empty) or 404
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4461842 to
11bad78
Compare
Contributor
Author
|
Some good follow ups would be to emit our own categories & top picks based on Grimmory information. |
be5dbde to
0b770a8
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.ts (1)
133-142:⚠️ Potential issue | 🟠 MajorMissing assignment for
forwardToKoboStoreinloadKoboAdminSettings().The new
forwardToKoboStorefield is initialized tofalsebut never loaded from the persisted settings. The toggle will always appear as OFF when the page loads, regardless of the actual saved value.🐛 Proposed fix
private loadKoboAdminSettings() { const settings = this.appSettingsService.appSettings(); if (settings) { this.koboSettings.convertToKepub = settings.koboSettings?.convertToKepub ?? true; this.koboSettings.conversionLimitInMb = settings.koboSettings?.conversionLimitInMb ?? 100; this.koboSettings.convertCbxToEpub = settings.koboSettings?.convertCbxToEpub ?? false; this.koboSettings.conversionLimitInMbForCbx = settings.koboSettings?.conversionLimitInMbForCbx ?? 100; this.koboSettings.forceEnableHyphenation = settings.koboSettings?.forceEnableHyphenation ?? false; this.koboSettings.conversionImageCompressionPercentage = settings.koboSettings?.conversionImageCompressionPercentage ?? 85; + this.koboSettings.forwardToKoboStore = settings.koboSettings?.forwardToKoboStore ?? false; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.ts` around lines 133 - 142, In loadKoboAdminSettings(), the persisted forwardToKoboStore value is not being loaded into the component state; update the method to read settings.koboSettings?.forwardToKoboStore and assign it to this.koboSettings.forwardToKoboStore (using the existing default fallback, e.g., ?? false) so the toggle reflects the saved setting; reference the loadKoboAdminSettings function, this.koboSettings, and appSettingsService.appSettings() when making the change.
🧹 Nitpick comments (3)
booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java (1)
31-106: Consider makinginitializationResourcesastatic finalfield.Since this map is immutable (created via
Map.ofEntries) and doesn't depend on instance state, declaring it asprivate static finalwould clarify intent and ensure it's only initialized once per class rather than per instance.♻️ Suggested change
- private final Map<String, String[]> initializationResources = Map.<String, String[]>ofEntries( + private static final Map<String, String[]> INITIALIZATION_RESOURCES = Map.<String, String[]>ofEntries(Then update the reference on line 133:
- for (String name : initializationResources.keySet()) { - resources.put(name, koboUrlBuilder.withBaseUrl(token, initializationResources.get(name))); + for (var entry : INITIALIZATION_RESOURCES.entrySet()) { + resources.put(entry.getKey(), koboUrlBuilder.withBaseUrl(token, entry.getValue())); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java` around lines 31 - 106, The map field initializationResources in KoboInitializationService should be declared private static final since it's immutable and class-wide; change its declaration to private static final Map<String,String[]> initializationResources and keep the Map.ofEntries initializer as-is, then update any uses in methods (e.g., any references within KoboInitializationService) to access the now-static field (no instance change needed) to ensure single-time class initialization.booklore-api/src/test/java/org/booklore/util/kobo/KoboUrlBuilderTest.java (1)
52-60: Unused variabletoken.The variable
tokenon line 55 is declared but not used; line 57 passes the string literal directly.♻️ Suggested cleanup
`@Test` void testImageUrlQualityTemplate() { String expected = "http://localhost:6060/api/kobo/testToken/v1/books/{ImageId}/thumbnail/{Width}/{Height}/{Quality}/{IsGreyscale}/image.jpg"; - String token = "testToken"; String actual = koboUrlBuilder.imageUrlQualityTemplate("testToken"); assertEquals(expected, actual); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/test/java/org/booklore/util/kobo/KoboUrlBuilderTest.java` around lines 52 - 60, The test method testImageUrlQualityTemplate declares an unused local variable token but calls koboUrlBuilder.imageUrlQualityTemplate with a string literal; either remove the unused token declaration or replace the literal with the token variable so it’s used: update the call in testImageUrlQualityTemplate to pass the token variable (or delete the token variable if you prefer the literal) to ensure no unused local remains.booklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.java (1)
172-174: Remove debug artifacts from test code.The
System.out.printlnstatement appears to be a leftover debug artifact and should be removed. Additionally, prefer JUnit'sassertNotNull(body)over the plainassertkeyword, which can be disabled at runtime.♻️ Proposed fix
var body = response.getBody(); - assert body != null; - System.out.println( body.getResources().get("example")); + assertNotNull(body); assertEquals("http://127.0.0.1/v1/library/sync", body.getResources().get("example").asText());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.java` around lines 172 - 174, In KoboInitializationServiceTest remove the debug System.out.println(...) call and replace the runtime-disabled Java assert with JUnit's assertion: change "assert body != null;" to use Assertions.assertNotNull(body) (or a static import of assertNotNull) so the test reliably fails when body is null; ensure the test imports org.junit.jupiter.api.Assertions (or the static method) and consider also asserting the specific resource (body.getResources().get("example")) if needed for test completeness.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@frontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.ts`:
- Around line 133-142: In loadKoboAdminSettings(), the persisted
forwardToKoboStore value is not being loaded into the component state; update
the method to read settings.koboSettings?.forwardToKoboStore and assign it to
this.koboSettings.forwardToKoboStore (using the existing default fallback, e.g.,
?? false) so the toggle reflects the saved setting; reference the
loadKoboAdminSettings function, this.koboSettings, and
appSettingsService.appSettings() when making the change.
---
Nitpick comments:
In
`@booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java`:
- Around line 31-106: The map field initializationResources in
KoboInitializationService should be declared private static final since it's
immutable and class-wide; change its declaration to private static final
Map<String,String[]> initializationResources and keep the Map.ofEntries
initializer as-is, then update any uses in methods (e.g., any references within
KoboInitializationService) to access the now-static field (no instance change
needed) to ensure single-time class initialization.
In
`@booklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.java`:
- Around line 172-174: In KoboInitializationServiceTest remove the debug
System.out.println(...) call and replace the runtime-disabled Java assert with
JUnit's assertion: change "assert body != null;" to use
Assertions.assertNotNull(body) (or a static import of assertNotNull) so the test
reliably fails when body is null; ensure the test imports
org.junit.jupiter.api.Assertions (or the static method) and consider also
asserting the specific resource (body.getResources().get("example")) if needed
for test completeness.
In `@booklore-api/src/test/java/org/booklore/util/kobo/KoboUrlBuilderTest.java`:
- Around line 52-60: The test method testImageUrlQualityTemplate declares an
unused local variable token but calls koboUrlBuilder.imageUrlQualityTemplate
with a string literal; either remove the unused token declaration or replace the
literal with the token variable so it’s used: update the call in
testImageUrlQualityTemplate to pass the token variable (or delete the token
variable if you prefer the literal) to ensure no unused local remains.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f8ba6344-bfb5-413c-800c-8aece65f625d
📒 Files selected for processing (12)
booklore-api/src/main/java/org/booklore/controller/KoboController.javabooklore-api/src/main/java/org/booklore/model/dto/settings/KoboSettings.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboResourcesComponent.javabooklore-api/src/main/java/org/booklore/util/kobo/KoboUrlBuilder.javabooklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.javabooklore-api/src/test/java/org/booklore/util/kobo/KoboUrlBuilderTest.javafrontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.htmlfrontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.tsfrontend/src/app/shared/model/app-settings.model.tsfrontend/src/i18n/en/settings-device.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
frontend/src/**/*.{ts,tsx,html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation in TypeScript, HTML, and SCSS files
Files:
frontend/src/app/shared/model/app-settings.model.tsfrontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.htmlfrontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.ts
frontend/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/src/app/**/*.{ts,tsx}: Preferinject()over constructor injection
Followfrontend/eslint.config.js: component selectors useapp-*, directive selectors useapp*, andanyis disallowed
Files:
frontend/src/app/shared/model/app-settings.model.tsfrontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.ts
booklore-api/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
booklore-api/src/**/*.java: Use 4-space indentation in Java files and match surrounding Java style
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection
Use MapStruct for entity/DTO mapping
Files:
booklore-api/src/main/java/org/booklore/model/dto/settings/KoboSettings.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboResourcesComponent.javabooklore-api/src/test/java/org/booklore/util/kobo/KoboUrlBuilderTest.javabooklore-api/src/main/java/org/booklore/controller/KoboController.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.javabooklore-api/src/main/java/org/booklore/util/kobo/KoboUrlBuilder.javabooklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.java
frontend/src/i18n/**
📄 CodeRabbit inference engine (AGENTS.md)
Put user-facing strings in Transloco files under
frontend/src/i18n/
Files:
frontend/src/i18n/en/settings-device.json
booklore-api/src/test/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Prefer focused unit tests; use
@SpringBootTestonly when the Spring context is required
Files:
booklore-api/src/test/java/org/booklore/util/kobo/KoboUrlBuilderTest.javabooklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.java
🧠 Learnings (9)
📓 Common learnings
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 198
File: booklore-api/src/main/java/org/booklore/controller/KoboController.java:100-103
Timestamp: 2026-03-26T02:20:27.862Z
Learning: In `booklore-api/src/main/java/org/booklore/controller/KoboController.java`, the Kobo reader device already makes direct outbound connections to Kobo's CDN/servers for other requests (confirmed by imnotjames via testing). Therefore, returning a `307 TEMPORARY_REDIRECT` to `cdn.kobo.com` for non-`BL-` book cover thumbnails (instead of server-side proxying) does NOT introduce a new privacy or behavioral regression — the Kobo device's IP and User-Agent are already exposed to Kobo through other direct requests.
📚 Learning: 2026-03-31T19:52:51.386Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 306
File: frontend/src/app/features/settings/user-management/user.service.ts:64-75
Timestamp: 2026-03-31T19:52:51.386Z
Learning: In `grimmory-tools/grimmory`, CBX reader settings (including `CbxPageViewMode`, `CbxPageSplitOption`, `CbxScrollMode`, etc.) are stored as a raw JSON string blob in the `user_settings` table under a setting key (e.g., `CBX_READER_SETTING`). The backend does not deserialize these values into typed Java enum instances, so adding new frontend enum values (e.g., `TWO_PAGE_REVERSED`, `CbxPageSplitOption` members) does NOT cause Jackson `InvalidFormatException` errors. Do not flag frontend-only enum additions as backend serialization issues for these settings.
Applied to files:
booklore-api/src/main/java/org/booklore/model/dto/settings/KoboSettings.java
📚 Learning: 2026-03-27T15:06:25.913Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 217
File: booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java:151-153
Timestamp: 2026-03-27T15:06:25.913Z
Learning: In `booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java`, the try/catch around `koboServerProxy.proxyCurrentRequest()` in `syncLibrary()` is intentionally scoped only to handle a missing/failed proxy response (null). Once a response is successfully obtained, its body and headers (including the upstream sync token decoded via `tokenGenerator.fromBase64()`) are processed without additional defensive wrapping. This is a deliberate design decision: other parts of the response are equally unguarded, and the goal is to handle proxy unavailability gracefully — not to catch every possible downstream processing error.
Applied to files:
booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.javabooklore-api/src/main/java/org/booklore/controller/KoboController.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.javabooklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.java
📚 Learning: 2026-03-25T19:09:09.638Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 189
File: booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java:113-116
Timestamp: 2026-03-25T19:09:09.638Z
Learning: In `booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java`, when deciding whether to forward the `Authorization` header to the upstream Kobo server proxy on the `/v1/library/sync` endpoint, a simple null check (`request.getHeader(HttpHeaders.AUTHORIZATION) != null`) is the correct guard. The header is either absent or present — if present it may or may not be a valid token, but validity is determined by the upstream Kobo server, not by Grimmory. Blank/whitespace intermediate states do not occur in practice, and pre-validating the token value before forwarding is not the responsibility of this service.
Applied to files:
booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.javabooklore-api/src/main/java/org/booklore/controller/KoboController.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java
📚 Learning: 2026-03-31T14:12:39.521Z
Learnt from: alexhb1
Repo: grimmory-tools/grimmory PR: 307
File: frontend/src/i18n/hr/layout.json:3-3
Timestamp: 2026-03-31T14:12:39.521Z
Learning: In `frontend/src/i18n/hr/layout.json` (Croatian locale), newly added i18n keys may intentionally use English placeholder values (e.g., `toggleNavigation`, `adjustSidebarWidth`, `closeNavigation`). Do not flag these as missing Croatian translations — this is a deliberate placeholder approach by the team.
Applied to files:
frontend/src/i18n/en/settings-device.json
📚 Learning: 2026-03-26T02:20:27.862Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 198
File: booklore-api/src/main/java/org/booklore/controller/KoboController.java:100-103
Timestamp: 2026-03-26T02:20:27.862Z
Learning: In `booklore-api/src/main/java/org/booklore/controller/KoboController.java`, the Kobo reader device already makes direct outbound connections to Kobo's CDN/servers for other requests (confirmed by imnotjames via testing). Therefore, returning a `307 TEMPORARY_REDIRECT` to `cdn.kobo.com` for non-`BL-` book cover thumbnails (instead of server-side proxying) does NOT introduce a new privacy or behavioral regression — the Kobo device's IP and User-Agent are already exposed to Kobo through other direct requests.
Applied to files:
booklore-api/src/test/java/org/booklore/util/kobo/KoboUrlBuilderTest.javabooklore-api/src/main/java/org/booklore/controller/KoboController.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java
📚 Learning: 2026-03-26T05:52:05.818Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 217
File: booklore-api/src/main/java/org/booklore/service/kobo/KoboServerProxy.java:107-108
Timestamp: 2026-03-26T05:52:05.818Z
Learning: In `booklore-api/src/main/java/org/booklore/service/kobo/KoboServerProxy.java`, the `executeProxyRequest()` method unconditionally parses every proxy response body as JSON via `objectMapper.readTree(response.body())`. Therefore, setting `Accept: application/json` on all outbound proxy requests is intentional and consistent — non-JSON responses from Kobo would already cause a Jackson parse exception. The Accept header is forced to `application/json` to encourage Kobo's server to return JSON-formatted error responses (rather than HTML), preventing parse failures on error paths like during sync.
Applied to files:
booklore-api/src/main/java/org/booklore/controller/KoboController.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.javabooklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.java
📚 Learning: 2026-03-24T18:46:47.249Z
Learnt from: pedronave
Repo: grimmory-tools/grimmory PR: 173
File: booklore-api/src/main/java/org/booklore/service/hardcover/HardcoverSyncService.java:188-199
Timestamp: 2026-03-24T18:46:47.249Z
Learning: In grimmory-tools/grimmory (HardcoverSyncService.java), when a `hardcoverBookId` is stored in book metadata but is malformed (non-numeric), the preferred behavior is to return null and skip the sync rather than falling back to ISBN-based lookup. The reasoning is that an explicitly set bookId represents deliberate user intent, and silently resolving via ISBN could sync progress to a different book than intended.
Applied to files:
booklore-api/src/main/java/org/booklore/controller/KoboController.java
📚 Learning: 2026-03-26T01:46:48.863Z
Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to booklore-api/src/test/**/*.java : Prefer focused unit tests; use `SpringBootTest` only when the Spring context is required
Applied to files:
booklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.java
🔇 Additional comments (21)
frontend/src/app/shared/model/app-settings.model.ts (1)
154-154: LGTM!Clean interface extension matching the backend DTO field.
frontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.html (1)
312-329: LGTM!The new toggle follows the established pattern in this component, with proper two-way binding and i18n key usage.
booklore-api/src/main/java/org/booklore/model/dto/settings/KoboSettings.java (1)
27-28: LGTM!The new field correctly uses
@Builder.Defaultand@JsonSetter(nulls = Nulls.SKIP), consistent with other fields in this class. The defaultfalsealigns with the PR's intent to disable Kobo Store forwarding by default.frontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.ts (1)
55-56: LGTM!Initialization follows the existing pattern with correct default value.
frontend/src/i18n/en/settings-device.json (1)
105-107: LGTM!The new i18n keys clearly describe the feature and match the template bindings.
booklore-api/src/main/java/org/booklore/service/kobo/KoboResourcesComponent.java (2)
15-16: LGTM!Returning
ObjectNodeinstead ofJsonNodeprovides proper typing for callers that need to modify the resources. The cast is safe since the embedded JSON is a known object literal.
95-114: LGTM!Disabling store-dependent features (instapaper, audiobook subscriptions, wishlist, etc.) in the fallback resources is appropriate when the Kobo Store proxy is disabled.
booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java (1)
111-129: LGTM!The conditional proxy logic with graceful fallback to local resources is well-implemented. The pattern matching
instanceof ObjectNode objectNodeand exception handling with logging provide robust error recovery.booklore-api/src/test/java/org/booklore/util/kobo/KoboUrlBuilderTest.java (1)
34-218: LGTM!Comprehensive test coverage for URL building, including edge cases for reverse proxy headers, port handling, and IP/domain scenarios.
booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java (4)
14-14: LGTM!The
AppSettingServiceis correctly added as a final field and will be injected via the existing@AllArgsConstructorLombok annotation, following the codebase's constructor injection pattern.Also applies to: 42-42
69-72: LGTM!Clean helper method that centralizes the forwarding decision. Since
getAppSettings()is decorated with@Cacheable("appSettings")(as shown in the relevant code snippet fromAppSettingService.java:45-48), repeated calls on each Kobo request will be served from cache, so there's no performance concern.
75-75: LGTM!The return type change from
ResponseEntity<?>toResponseEntity<List<Entitlement>>improves type safety and aligns with the actual return value.
144-164: LGTM!The proxy call and entitlement augmentation are now correctly gated by
isForwardingToKoboStore(). When forwarding is disabled, the sync completes with only local entitlements. The existing try/catch for proxy failures is intentionally scoped per the learnings, and this change preserves that design.booklore-api/src/main/java/org/booklore/controller/KoboController.java (4)
45-45: LGTM!The
AppSettingServiceis correctly injected via@RequiredArgsConstructor, and theisForwardingToKoboStore()helper centralizes the forwarding decision logic consistently withKoboLibrarySyncService.Also applies to: 57-59
105-112: LGTM!When forwarding is disabled, non-local thumbnails (
BL-prefix absent) correctly throwGENERIC_NOT_FOUNDinstead of redirecting tocdn.kobo.com. This aligns with the PR objective to disable external Kobo Store interactions when the feature is off.
128-132: LGTM!All book-related endpoints (metadata, state, download, delete) consistently gate proxy calls with
isForwardingToKoboStore()and throwGENERIC_NOT_FOUNDfor non-numeric book IDs when forwarding is disabled. The pattern is uniform across all endpoints.Also applies to: 141-145, 156-160, 179-183, 196-200
206-222: LGTM!The catch-all route correctly:
- Always blocks analytics events (never forwarded, as intended)
- Always returns empty 200 for nextread patterns
- Proxies to Kobo store only when forwarding is enabled
- Returns empty 200 when forwarding is disabled (per PR objectives: "unknown Kobo requests receive a 200 OK empty response")
booklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.java (2)
28-28: LGTM!The
LENIENTstrictness is appropriate here sincemockWithForwardingToKoboStore()is called insetUp()but not all tests depend on that behavior. The helper method cleanly builds the settings hierarchy for testing different forwarding states.Also applies to: 50-60
153-176: Good test coverage for the new forwarding flag.This test properly verifies the fallback behavior when
forwardToKoboStoreis disabled, ensuring local resources are returned instead of proxied resources.booklore-api/src/main/java/org/booklore/util/kobo/KoboUrlBuilder.java (2)
47-58: LGTM!Good DRY refactor. The
withBaseUrl()helper consolidates the common pattern of building a base URL with the Kobo API path prefix (/api/kobo/{token}) and additional path segments. This reduces duplication acrossdownloadUrl(),imageUrlTemplate(), andimageUrlQualityTemplate().
60-70: LGTM!All three methods (
downloadUrl,imageUrlTemplate,imageUrlQualityTemplate) now cleanly delegate towithBaseUrl(), eliminating duplicate builder logic. The output remains functionally equivalent for consumers likeKoboEntitlementService(which usesdownloadUrldirectly as a URL string per the relevant code snippet).
bac4dee to
861f913
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java (1)
27-102: Avoid maintaining this override list separately fromKoboResourcesComponent.This new map duplicates the resource identifiers already hard-coded in
booklore-api/src/main/java/org/booklore/service/kobo/KoboResourcesComponent.java:15-187. Any future addition or rename now has to be updated in two places or some initialization resources will silently stop being rewritten to Grimmory URLs. A shared descriptor/source of truth would make this much harder to drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java` around lines 27 - 102, KoboInitializationService currently defines a duplicate static map initializationResources that mirrors the canonical resource list in KoboResourcesComponent; remove this duplicated map and instead read/derive the initialization resources from the single source-of-truth in KoboResourcesComponent (e.g., reference its public static map or call its accessor method), updating any code that used initializationResources to use KoboResourcesComponent (or inject/instantiate KoboResourcesComponent) so future changes are only made in one place.frontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.html (1)
313-327: Ship the matching docs update with this toggle.The linked
grimmory-tools/grimmory-docsfindings still pointdocs/integration/kobo.md,docs/tools/devices.md, and the Kobo admin-settings screenshot at the pre-toggle UX. Without that follow-up, admins will not see that forwarding is now configurable and defaults to off.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.html` around lines 313 - 327, Update the external docs to reflect the new forwarding toggle added in the Kobo settings UI: document that the p-toggle-switch with id "forwardToKoboStore" (bound to koboSettings.forwardToKoboStore in kobo-sync-settings-component) makes forwarding configurable and now defaults to off, update docs/integration/kobo.md and docs/tools/devices.md to include the new configuration and default behavior, and replace the Kobo admin-settings screenshot showing the pre-toggle UX with one that shows the toggle and its off state; ensure copy references the i18n key t('kobo.forwardToKoboStore') where appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@booklore-api/src/main/java/org/booklore/controller/KoboController.java`:
- Around line 176-183: The downloadBook method currently discards forwarded
proxy responses; change the signature from void to ResponseEntity<JsonNode> and
in the branch where isForwardingToKoboStore() is true return the result of
koboServerProxy.proxyCurrentRequest(null, false) instead of calling it and
dropping the value, while keeping the existing path that calls
bookDownloadService.downloadKoboBook(Long.parseLong(bookId), response) for
numeric bookId and still throwing ApiError.GENERIC_NOT_FOUND for the else case
so callers receive the proxied ResponseEntity.
In
`@booklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.java`:
- Line 28: The test currently uses `@MockitoSettings`(strictness =
Strictness.LENIENT) and stubs koboServerProxy.proxyCurrentRequest() even though
forwardToKoboStore is set false and the KoboInitializationService code path
skips proxy usage; remove the unnecessary lenient setting and the unused stub,
replace the weak Java assert with JUnit's assertNotNull(...) on the fallback
result, and add verifyNoInteractions(koboServerProxy) to assert the proxy was
not called; update the test that references forwardToKoboStore,
koboServerProxy.proxyCurrentRequest(), and KoboInitializationService to reflect
these changes.
---
Nitpick comments:
In
`@booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java`:
- Around line 27-102: KoboInitializationService currently defines a duplicate
static map initializationResources that mirrors the canonical resource list in
KoboResourcesComponent; remove this duplicated map and instead read/derive the
initialization resources from the single source-of-truth in
KoboResourcesComponent (e.g., reference its public static map or call its
accessor method), updating any code that used initializationResources to use
KoboResourcesComponent (or inject/instantiate KoboResourcesComponent) so future
changes are only made in one place.
In
`@frontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.html`:
- Around line 313-327: Update the external docs to reflect the new forwarding
toggle added in the Kobo settings UI: document that the p-toggle-switch with id
"forwardToKoboStore" (bound to koboSettings.forwardToKoboStore in
kobo-sync-settings-component) makes forwarding configurable and now defaults to
off, update docs/integration/kobo.md and docs/tools/devices.md to include the
new configuration and default behavior, and replace the Kobo admin-settings
screenshot showing the pre-toggle UX with one that shows the toggle and its off
state; ensure copy references the i18n key t('kobo.forwardToKoboStore') where
appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1686db7e-85bd-4cdc-af05-3de046c9359f
📒 Files selected for processing (12)
booklore-api/src/main/java/org/booklore/controller/KoboController.javabooklore-api/src/main/java/org/booklore/model/dto/settings/KoboSettings.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboResourcesComponent.javabooklore-api/src/main/java/org/booklore/util/kobo/KoboUrlBuilder.javabooklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.javabooklore-api/src/test/java/org/booklore/util/kobo/KoboUrlBuilderTest.javafrontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.htmlfrontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.tsfrontend/src/app/shared/model/app-settings.model.tsfrontend/src/i18n/en/settings-device.json
✅ Files skipped from review due to trivial changes (2)
- frontend/src/app/shared/model/app-settings.model.ts
- booklore-api/src/main/java/org/booklore/model/dto/settings/KoboSettings.java
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.ts
- booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java
- booklore-api/src/main/java/org/booklore/service/kobo/KoboResourcesComponent.java
- frontend/src/i18n/en/settings-device.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (3)
frontend/src/**/*.{ts,tsx,html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation in TypeScript, HTML, and SCSS files
Files:
frontend/src/app/features/settings/device-settings/component/kobo-sync-settings/kobo-sync-settings-component.html
booklore-api/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
booklore-api/src/**/*.java: Use 4-space indentation in Java files and match surrounding Java style
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection
Use MapStruct for entity/DTO mapping
Files:
booklore-api/src/main/java/org/booklore/controller/KoboController.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.javabooklore-api/src/main/java/org/booklore/util/kobo/KoboUrlBuilder.javabooklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.javabooklore-api/src/test/java/org/booklore/util/kobo/KoboUrlBuilderTest.java
booklore-api/src/test/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Prefer focused unit tests; use
@SpringBootTestonly when the Spring context is required
Files:
booklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.javabooklore-api/src/test/java/org/booklore/util/kobo/KoboUrlBuilderTest.java
🧠 Learnings (7)
📓 Common learnings
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 198
File: booklore-api/src/main/java/org/booklore/controller/KoboController.java:100-103
Timestamp: 2026-03-26T02:20:27.862Z
Learning: In `booklore-api/src/main/java/org/booklore/controller/KoboController.java`, the Kobo reader device already makes direct outbound connections to Kobo's CDN/servers for other requests (confirmed by imnotjames via testing). Therefore, returning a `307 TEMPORARY_REDIRECT` to `cdn.kobo.com` for non-`BL-` book cover thumbnails (instead of server-side proxying) does NOT introduce a new privacy or behavioral regression — the Kobo device's IP and User-Agent are already exposed to Kobo through other direct requests.
📚 Learning: 2026-03-26T02:20:27.862Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 198
File: booklore-api/src/main/java/org/booklore/controller/KoboController.java:100-103
Timestamp: 2026-03-26T02:20:27.862Z
Learning: In `booklore-api/src/main/java/org/booklore/controller/KoboController.java`, the Kobo reader device already makes direct outbound connections to Kobo's CDN/servers for other requests (confirmed by imnotjames via testing). Therefore, returning a `307 TEMPORARY_REDIRECT` to `cdn.kobo.com` for non-`BL-` book cover thumbnails (instead of server-side proxying) does NOT introduce a new privacy or behavioral regression — the Kobo device's IP and User-Agent are already exposed to Kobo through other direct requests.
Applied to files:
booklore-api/src/main/java/org/booklore/controller/KoboController.javabooklore-api/src/test/java/org/booklore/util/kobo/KoboUrlBuilderTest.java
📚 Learning: 2026-03-27T15:06:25.913Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 217
File: booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java:151-153
Timestamp: 2026-03-27T15:06:25.913Z
Learning: In `booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java`, the try/catch around `koboServerProxy.proxyCurrentRequest()` in `syncLibrary()` is intentionally scoped only to handle a missing/failed proxy response (null). Once a response is successfully obtained, its body and headers (including the upstream sync token decoded via `tokenGenerator.fromBase64()`) are processed without additional defensive wrapping. This is a deliberate design decision: other parts of the response are equally unguarded, and the goal is to handle proxy unavailability gracefully — not to catch every possible downstream processing error.
Applied to files:
booklore-api/src/main/java/org/booklore/controller/KoboController.javabooklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.java
📚 Learning: 2026-03-25T19:09:09.638Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 189
File: booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java:113-116
Timestamp: 2026-03-25T19:09:09.638Z
Learning: In `booklore-api/src/main/java/org/booklore/service/kobo/KoboLibrarySyncService.java`, when deciding whether to forward the `Authorization` header to the upstream Kobo server proxy on the `/v1/library/sync` endpoint, a simple null check (`request.getHeader(HttpHeaders.AUTHORIZATION) != null`) is the correct guard. The header is either absent or present — if present it may or may not be a valid token, but validity is determined by the upstream Kobo server, not by Grimmory. Blank/whitespace intermediate states do not occur in practice, and pre-validating the token value before forwarding is not the responsibility of this service.
Applied to files:
booklore-api/src/main/java/org/booklore/controller/KoboController.java
📚 Learning: 2026-03-24T18:46:47.249Z
Learnt from: pedronave
Repo: grimmory-tools/grimmory PR: 173
File: booklore-api/src/main/java/org/booklore/service/hardcover/HardcoverSyncService.java:188-199
Timestamp: 2026-03-24T18:46:47.249Z
Learning: In grimmory-tools/grimmory (HardcoverSyncService.java), when a `hardcoverBookId` is stored in book metadata but is malformed (non-numeric), the preferred behavior is to return null and skip the sync rather than falling back to ISBN-based lookup. The reasoning is that an explicitly set bookId represents deliberate user intent, and silently resolving via ISBN could sync progress to a different book than intended.
Applied to files:
booklore-api/src/main/java/org/booklore/controller/KoboController.java
📚 Learning: 2026-03-26T05:52:05.818Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 217
File: booklore-api/src/main/java/org/booklore/service/kobo/KoboServerProxy.java:107-108
Timestamp: 2026-03-26T05:52:05.818Z
Learning: In `booklore-api/src/main/java/org/booklore/service/kobo/KoboServerProxy.java`, the `executeProxyRequest()` method unconditionally parses every proxy response body as JSON via `objectMapper.readTree(response.body())`. Therefore, setting `Accept: application/json` on all outbound proxy requests is intentional and consistent — non-JSON responses from Kobo would already cause a Jackson parse exception. The Accept header is forced to `application/json` to encourage Kobo's server to return JSON-formatted error responses (rather than HTML), preventing parse failures on error paths like during sync.
Applied to files:
booklore-api/src/main/java/org/booklore/controller/KoboController.javabooklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.java
📚 Learning: 2026-03-26T01:46:48.863Z
Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to booklore-api/src/test/**/*.java : Prefer focused unit tests; use `SpringBootTest` only when the Spring context is required
Applied to files:
booklore-api/src/test/java/org/booklore/service/kobo/KoboInitializationServiceTest.java
🔀 Multi-repo context grimmory-tools/grimmory-docs
Findings — grimmory-tools/grimmory-docs
-
Docs reference Kobo integration, admin settings, and the device config flow; these should be updated to mention the new "Forward to Rakuten Kobo Store" admin toggle and any changed behavior:
- docs/integration/kobo.md — full Kobo integration guide (device config, admin settings, troubleshooting). [::grimmory-tools/grimmory-docs::docs/integration/kobo.md]
- docs/tools/devices.md — Devices page and "⚙️ Kobo Administrator Settings" section that shows the Kobo admin screenshot and lists admin-level Kobo options. [::grimmory-tools/grimmory-docs::docs/tools/devices.md]
- docs/tools/user-management.md — feature table includes "Kobo Sync". [::grimmory-tools/grimmory-docs::docs/tools/user-management.md]
-
Images/screenshots referenced by docs that likely need updating to show the new toggle:
- static/img/devices/kobo-admin-settings.jpg — used in docs/tools/devices.md. [::grimmory-tools/grimmory-docs::static/img/devices/kobo-admin-settings.jpg]
- static/img/kobo/* — many Kobo-related images used across docs (e.g., device-settings, config-before/after, sync-button). Review these for any UI changes to the admin settings. [::grimmory-tools/grimmory-docs::static/img/kobo/]
-
Site navigation includes a Kobo entry; docs/site structure will surface the updated text/screenshots:
- sidebars.ts — includes integration/kobo in the docs sidebar. [::grimmory-tools/grimmory-docs::sidebars.ts]
- src/pages/HomepageFeatures.tsx — mentions "Kobo & KOReader Sync" on the homepage features list. [::grimmory-tools/grimmory-docs::src/pages/HomepageFeatures.tsx]
Conclusion: Documentation repository contains multiple consumer-facing pages and images referencing Kobo admin settings and device behavior that should be updated to document the new forwardToKoboStore setting and its effects (including default OFF and changed proxy/redirect behavior).
imnotjames
commented
Apr 2, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java (1)
38-40: Please ship the docs update with this toggle.This changes a visible admin setting and Kobo initialization behavior. Update
grimmory-tools/grimmory-docsalongside this (docs/integration/kobo.md,docs/tools/devices.md, and the admin-settings screenshot) so admins know forwarding is now opt-in and defaults OFF.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java` around lines 38 - 40, The new toggle in KoboInitializationService.isForwardingToKoboStore (calls appSettingService.getAppSettings().getKoboSettings().isForwardToKoboStore()) makes forwarding to the Kobo store opt-in and defaults OFF; update the public docs in the grimmory-docs repo (kobo.md, devices.md and the admin-settings screenshot) to reflect the new default and admin workflow for enabling forwarding, and include a short note in the integration/setup section explaining the behavior change and where admins can toggle it in the UI.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java`:
- Around line 38-40: The new gate uses KoboSettings.forwardToKoboStore which
defaults to false and will silently disable forwarding for existing AppSettings
that lack the field; update the code and data migration: either add a one-time
migration/backfill that sets forwardToKoboStore=true for existing persisted
AppSettings, or change isForwardingToKoboStore() to treat a missing/null
forwardToKoboStore as "enabled" for legacy records by checking
appSettingService.getAppSettings().getKoboSettings().getForwardToKoboStore() for
null and returning true in that case (and add a debug log warning so admins can
explicitly set the new flag later).
---
Nitpick comments:
In
`@booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java`:
- Around line 38-40: The new toggle in
KoboInitializationService.isForwardingToKoboStore (calls
appSettingService.getAppSettings().getKoboSettings().isForwardToKoboStore())
makes forwarding to the Kobo store opt-in and defaults OFF; update the public
docs in the grimmory-docs repo (kobo.md, devices.md and the admin-settings
screenshot) to reflect the new default and admin workflow for enabling
forwarding, and include a short note in the integration/setup section explaining
the behavior change and where admins can toggle it in the UI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 025dd923-6c12-4abf-b37b-0e340bf0ae60
📒 Files selected for processing (2)
booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.javabooklore-api/src/main/java/org/booklore/service/kobo/KoboResourcesComponent.java
🚧 Files skipped from review as they are similar to previous changes (1)
- booklore-api/src/main/java/org/booklore/service/kobo/KoboResourcesComponent.java
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (1)
booklore-api/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
booklore-api/src/**/*.java: Use 4-space indentation in Java files and match surrounding Java style
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection
Use MapStruct for entity/DTO mapping
Files:
booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java
🧠 Learnings (1)
📓 Common learnings
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 198
File: booklore-api/src/main/java/org/booklore/controller/KoboController.java:100-103
Timestamp: 2026-03-26T02:20:27.862Z
Learning: In `booklore-api/src/main/java/org/booklore/controller/KoboController.java`, the Kobo reader device already makes direct outbound connections to Kobo's CDN/servers for other requests (confirmed by imnotjames via testing). Therefore, returning a `307 TEMPORARY_REDIRECT` to `cdn.kobo.com` for non-`BL-` book cover thumbnails (instead of server-side proxying) does NOT introduce a new privacy or behavioral regression — the Kobo device's IP and User-Agent are already exposed to Kobo through other direct requests.
🔀 Multi-repo context grimmory-tools/grimmory-docs
[::grimmory-tools/grimmory-docs::docs/integration/kobo.md]
- Contains the Kobo integration guide; multiple references to device configuration, admin settings, and images that surface the Kobo admin UI and behavior (e.g., api_endpoint instructions, troubleshooting, screenshots). Update to document the new "Forward to Rakuten Kobo Store" toggle, its default OFF behavior, and the changed proxy/redirect behavior when disabled. (seen lines in file)
[::grimmory-tools/grimmory-docs::docs/tools/devices.md]
- Devices page docs include a "⚙️ Kobo Administrator Settings" section and the screenshot reference /img/devices/kobo-admin-settings.jpg. The new admin toggle should be reflected here (table descriptions and screenshot). (seen lines in file)
[::grimmory-tools/grimmory-docs::docs/tools/user-management.md]
- User management docs list "Kobo Sync" in Device Sync permissions; consider whether permission/copy changes are needed to mention forwarding behavior. (seen lines in file)
[::grimmory-tools/grimmory-docs::static/img/devices/kobo-admin-settings.jpg]
- Binary image referenced by docs/tools/devices.md. Likely needs updating to include the new "Forward to Rakuten Kobo Store" toggle (PR adds UI toggle screenshots). (file present)
[::grimmory-tools/grimmory-docs::sidebars.ts]
- Sidebar includes integration/kobo entry; no structural change required but docs navigation will surface updated pages. (file present)
[::grimmory-tools/grimmory-docs::src/pages/HomepageFeatures.tsx]
- Marketing copy lists "Kobo & KOReader Sync" in feature tiles. Consider minor copy/update if public-facing messaging should mention the new optional forwarding behavior. (file present)
Conclusion: grimmory-docs contains several docs and an image that directly reference Kobo admin settings and device behavior; they should be updated to document the new forwardToKoboStore setting (default OFF) and the altered proxy/redirect responses when forwarding is disabled.
🔇 Additional comments (1)
booklore-api/src/main/java/org/booklore/service/kobo/KoboInitializationService.java (1)
45-63: Nice degradation path.Falling back to
KoboResourcesComponentwhen Kobo's/v1/initializationpayload is missing or malformed keeps device setup alive, and the returnedObjectNodeis fresh per request, so the laterput(...)calls stay request-local.
4ded29a to
f3b309c
Compare
this cuts down the advertised routes - in particular to reduce the number of auth-related requests we will be taking on. we may eventually want to do this but I want to reduce the amount of change in this PR
This reverts commit 1b242fa.
f3b309c to
5eeb52c
Compare
Member
|
@imnotjames As per discord, let me know if we're good for a final review here :) |
Contributor
Author
We should be good. |
zachyale
approved these changes
Apr 7, 2026
zachyale
left a comment
Member
There was a problem hiding this comment.
Thanks for focusing on this area of the platform so much recently- your domain expertise here is very much appreciated 👍
zachyale
pushed a commit
to zachyale/grimmory
that referenced
this pull request
Apr 17, 2026
* chore(api): fix typing for kobo sync service * feat(api): add more paths for grimmory/kobo API * fix(api): disable kobo features if kobo is unavailable * feat(api): add proxy flag to kobo setting * feat(api): adhere to kobo proxy enabled flag * feat(ui): add forward requests toggle * fix(ui): load setting * style: use assertNotNull * fix: reduce advertised routes this cuts down the advertised routes - in particular to reduce the number of auth-related requests we will be taking on. we may eventually want to do this but I want to reduce the amount of change in this PR * fix: undo disable of kobo features This reverts commit 1b242fa.
zachyale
pushed a commit
to zachyale/grimmory
that referenced
this pull request
Apr 17, 2026
* chore(api): fix typing for kobo sync service * feat(api): add more paths for grimmory/kobo API * fix(api): disable kobo features if kobo is unavailable * feat(api): add proxy flag to kobo setting * feat(api): adhere to kobo proxy enabled flag * feat(ui): add forward requests toggle * fix(ui): load setting * style: use assertNotNull * fix: reduce advertised routes this cuts down the advertised routes - in particular to reduce the number of auth-related requests we will be taking on. we may eventually want to do this but I want to reduce the amount of change in this PR * fix: undo disable of kobo features This reverts commit 1b242fa.
zachyale
pushed a commit
that referenced
this pull request
Apr 17, 2026
* chore(api): fix typing for kobo sync service * feat(api): add more paths for grimmory/kobo API * fix(api): disable kobo features if kobo is unavailable * feat(api): add proxy flag to kobo setting * feat(api): adhere to kobo proxy enabled flag * feat(ui): add forward requests toggle * fix(ui): load setting * style: use assertNotNull * fix: reduce advertised routes this cuts down the advertised routes - in particular to reduce the number of auth-related requests we will be taking on. we may eventually want to do this but I want to reduce the amount of change in this PR * fix: undo disable of kobo features This reverts commit 1b242fa.
zachyale
pushed a commit
that referenced
this pull request
Apr 22, 2026
* chore(api): fix typing for kobo sync service * feat(api): add more paths for grimmory/kobo API * fix(api): disable kobo features if kobo is unavailable * feat(api): add proxy flag to kobo setting * feat(api): adhere to kobo proxy enabled flag * feat(ui): add forward requests toggle * fix(ui): load setting * style: use assertNotNull * fix: reduce advertised routes this cuts down the advertised routes - in particular to reduce the number of auth-related requests we will be taking on. we may eventually want to do this but I want to reduce the amount of change in this PR * fix: undo disable of kobo features This reverts commit 1b242fa.
This was referenced Apr 30, 2026
dsmouse
pushed a commit
to dsmouse/grimmory
that referenced
this pull request
May 11, 2026
* chore(api): fix typing for kobo sync service * feat(api): add more paths for grimmory/kobo API * fix(api): disable kobo features if kobo is unavailable * feat(api): add proxy flag to kobo setting * feat(api): adhere to kobo proxy enabled flag * feat(ui): add forward requests toggle * fix(ui): load setting * style: use assertNotNull * fix: reduce advertised routes this cuts down the advertised routes - in particular to reduce the number of auth-related requests we will be taking on. we may eventually want to do this but I want to reduce the amount of change in this PR * fix: undo disable of kobo features This reverts commit 1b242fa.
This was referenced May 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Currently, the Kobo Sync will always forward requests to the Rakuten Kobo Store.
Adds a Kobo Sync setting to control the "forwarding" requests of unhandled endpoints to the Rakuten Kobo Store. This allows users to turn off requests to Rakuten's third party server.
Linked Issue: Fixes #193
Screenshots
UI Toggle
Kobo Reader Discover Tab
Changes
200 OKempty response.Summary by CodeRabbit
New Features
Improvements
Tests