refactor: outputs built templates into separate files for summary and templates#2643
refactor: outputs built templates into separate files for summary and templates#2643
Conversation
📝 WalkthroughWalkthroughThis PR refactors template caching from in-memory to disk-backed storage with per-template JSON files, adds a new automated Cloudflare Pages deployment workflow, updates HTTP client architecture to use dedicated baseURL configuration, introduces legacy .json route redirects, and adds environment-specific header handling for cross-origin requests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TemplateController
participant TemplateGalleryService
participant FileSystem as File System<br/>(disk cache)
participant LRUCache
rect rgba(100, 150, 200, 0.5)
Note over Client,LRUCache: New Disk-Backed Template Retrieval Flow
end
Client->>TemplateController: GET /v1/templates-list
TemplateController->>TemplateGalleryService: getGallerySummaryBuffer()
alt Cache hit (in-memory)
TemplateGalleryService->>LRUCache: Check `#templatesSummaryCache`
LRUCache-->>TemplateGalleryService: Return Buffer
else Cache miss
TemplateGalleryService->>FileSystem: Read /data/templates/v1/templates-list.json
FileSystem-->>TemplateGalleryService: Buffer content
TemplateGalleryService->>LRUCache: Store Buffer in `#templatesSummaryCache`
end
TemplateGalleryService-->>TemplateController: Buffer
TemplateController->>Client: Response (Content-Type: application/json)
sequenceDiagram
participant GithubScheduler as GitHub Scheduler
participant GithubActions as GitHub Actions
participant NPM
participant FileSystem as File System
participant Cloudflare as Cloudflare Pages
participant Wrangler
rect rgba(100, 150, 200, 0.5)
Note over GithubScheduler,Wrangler: Automated Template Build & Deploy Workflow
end
GithubScheduler->>GithubActions: Trigger daily at 15:00 UTC
GithubActions->>GithubActions: Checkout repository
GithubActions->>GithubActions: Setup Node.js via actions/setup-node@v6
GithubActions->>NPM: npm run build:akash-templates --workspace=apps/api
NPM->>FileSystem: Generate template files in apps/api/dist/.data/templates
GithubActions->>FileSystem: Delete generated files (excluding v1)
GithubActions->>FileSystem: Write _headers file
GithubActions->>Wrangler: wrangler pages deploy dist/.data/templates/
Wrangler->>Cloudflare: Deploy to akash-templates project
Cloudflare-->>Wrangler: Deployment confirmed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@apps/api/src/template/services/template-gallery/template-gallery.service.ts`:
- Around line 57-60: The refreshCache method currently rebuilds the gallery and
clears `#parsedTemplates` and templateFetcher archive cache but does not
invalidate `#templatesSummaryCache`, so getGallerySummaryBuffer() may return stale
data; update the refreshCache implementation (the async refreshCache that calls
buildTemplateGalleryCache) to also clear or reset `#templatesSummaryCache` after
buildTemplateGalleryCache completes (alongside this.#parsedTemplates.clear() and
this.templateFetcher?.clearArchiveCache()) so the summary buffer is regenerated
on next getGallerySummaryBuffer() call.
- Around line 81-87: The `#templateCachePath` function is interpolating templateId
directly into a filesystem path, allowing IDs with "/" or ".." to escape the
by-id directory; fix by validating or canonicalizing templateId inside
`#templateCachePath`: reject or sanitize any IDs containing path separators or
parent-segment tokens (e.g., disallow "/" and ".."), or encode the ID into a
safe form (URL-encode, base64/url-safe, or replace non-allowed chars with safe
tokens) before building `${this.#galleriesCachePath}/by-id/${...}.json`; keep
`#summaryCachePath` unchanged but ensure callers of `#templateCachePath` pass
validated IDs.
- Around line 52-55: getGallerySummaryBuffer currently assigns a pending Promise
to the private field `#templatesSummaryCache` using this.#fs.readFile(...), which
if it rejects will cache a rejected Promise and never retry; change the logic so
you await the read and on failure clear the cache before rethrowing or returning
a fallback. Concretely, replace the direct assignment with code that sets
`#templatesSummaryCache` to the read Promise only while awaiting it (or use a
.catch handler on the Promise from this.#fs.readFile(this.#summaryCachePath())
that sets this.#templatesSummaryCache = undefined and then throws the error), so
subsequent calls can attempt to reload the file; refer to the
getGallerySummaryBuffer method, the `#templatesSummaryCache` field, the
`#fs.readFile` call and `#summaryCachePath` to locate where to apply the change.
- Around line 134-140: The code currently assigns JSON.parse result to
`template` (implicitly any) and caches it in `#parsedTemplates`; replace this by
parsing then validating against a typed schema (e.g., a Zod schema or a
TypeScript type guard) for the Template shape before caching and returning.
Specifically, create/inline a `Template` schema or guard (or import one), call
`JSON.parse` into an untyped value, run schema.parse/validate or guard(value) to
assert the shape, throw or log and propagate an error if validation fails, and
only `this.#parsedTemplates.set(id, validatedTemplate)` and return the
strongly-typed `Template` when validation succeeds; update the method’s return
type to `Template` (or Promise<Template>) accordingly. In TypeScript, prefer
runtime validation libraries like Zod or explicit type guard functions to
validate JSON.parse results rather than casting to `any`.
apps/api/src/template/services/template-gallery/template-gallery.service.ts
Show resolved
Hide resolved
apps/api/src/template/services/template-gallery/template-gallery.service.ts
Show resolved
Hide resolved
apps/api/src/template/services/template-gallery/template-gallery.service.ts
Show resolved
Hide resolved
apps/api/src/template/services/template-gallery/template-gallery.service.ts
Show resolved
Hide resolved
ae2b268 to
49517e3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/api/src/template/services/template-gallery/template-gallery.service.ts`:
- Around line 63-79: The PromisePool call in buildTemplateGalleryCache can
swallow write errors because PromisePool.process() returns a result object with
an errors array instead of throwing; change the PromisePool invocation to
capture its return (e.g., const result = await
PromisePool.for(allTemplates).withConcurrency(100).process(...)) and then check
result.errors (or result.hasErrors) and surface them—either throw an aggregated
Error or log and rethrow—so failed `#fs.writeFile` operations for each template.id
are not silently ignored; ensure the method returns/throws appropriately after
inspecting result.errors.
🧹 Nitpick comments (1)
apps/api/src/template/services/template-gallery/template-gallery.service.spec.ts (1)
262-289: Align thesetuphelper with test guidelines.
setupshould be the last helper in the rootdescribeand accept a single parameter with inline type definition. Consider moving it belowcreateAsyncGenerator/createCategory, and adding an options param (even if unused).As per coding guidelines "Use `setup` function instead of `beforeEach` in test files. The `setup` function must be at the bottom of the root `describe` block, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type."🧩 Example signature tweak
-function setup() { +function setup({ dataFolderPath = "/data" }: { dataFolderPath?: string } = {}) { const logger = mock<LoggerService>(); const fsMock = mock<FileSystemApi>({ access: jest.fn(() => Promise.reject(new Error("File not found"))) }); - const dataFolderPath = "/data";
apps/api/src/template/services/template-gallery/template-gallery.service.ts
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (79.48%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2643 +/- ##
==========================================
- Coverage 51.37% 51.34% -0.03%
==========================================
Files 1053 1053
Lines 29593 29578 -15
Branches 6659 6651 -8
==========================================
- Hits 15204 15188 -16
+ Misses 14154 14003 -151
- Partials 235 387 +152
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
3d18cdf to
6f1f66c
Compare
6f1f66c to
ca185bf
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/deploy-web/src/services/app-di-container/app-di-container.ts`:
- Around line 71-81: The Content-Type deletion is targeting the wrong place and
is a no-op; update the template factory (the template: () => { ... } block that
creates httpClient via container.createAxios and returns new
TemplateHttpService) to remove the header from the correct locations: delete
httpClient.defaults.headers.common['Content-Type'] and also remove
'Content-Type' from any per-method header objects (e.g.
httpClient.defaults.headers.post/put/patch/delete or by iterating all keys of
httpClient.defaults.headers except 'common' and deleting ['Content-Type'] on
each). Keep the existing removal of Accept on defaults.headers.common. Ensure
these deletions occur only when config.runtimeEnv === "browser" before
constructing TemplateHttpService.
In `@packages/http-sdk/src/template/template-http.service.ts`:
- Around line 41-42: The findById method interpolates raw id into the request
path which can produce malformed URLs or path traversal; URL-encode the id
before interpolation (use encodeURIComponent on the id passed to
TemplateHttpService.findById) so the call to
this.#httpClient.get<TemplateOutput>(`/v1/templates/${...}.json`) uses the
encoded value when building the URL.
Why
To make the format compatible with what we will publish to cloudflare pages.
What
voltafield in package.jsonSummary by CodeRabbit
Release Notes
New Features
Chores