Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: redis weak #426

Merged
merged 1 commit into from
Mar 20, 2023
Merged

feat: redis weak #426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 30 additions & 21 deletions app/port/controller/package/ShowPackageController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,28 +31,35 @@ export class ShowPackageController extends AbstractController {
const isSync = isSyncWorkerRequest(ctx);
const abbreviatedMetaType = 'application/vnd.npm.install-v1+json';
const isFullManifests = ctx.accepts([ 'json', abbreviatedMetaType ]) !== abbreviatedMetaType;

// handle cache
const cacheEtag = await this.cacheService.getPackageEtag(fullname, isFullManifests);
if (!isSync && cacheEtag) {
let requestEtag = ctx.request.get('if-none-match');
if (requestEtag.startsWith('W/')) {
requestEtag = requestEtag.substring(2);
}
if (requestEtag === cacheEtag) {
// make sure CDN cache header set here
this.setCDNHeaders(ctx);
// match etag, set status 304
ctx.status = 304;
return;
}
// get cache pkg data
const cacheBytes = await this.cacheService.getPackageManifests(fullname, isFullManifests);
if (cacheBytes && cacheBytes.length > 0) {
ctx.set('etag', `W/${cacheEtag}`);
ctx.type = 'json';
this.setCDNHeaders(ctx);
return cacheBytes;
// fallback to db when cache error
try {
const cacheEtag = await this.cacheService.getPackageEtag(fullname, isFullManifests);
if (!isSync && cacheEtag) {
let requestEtag = ctx.request.get('if-none-match');
if (requestEtag.startsWith('W/')) {
requestEtag = requestEtag.substring(2);
}
if (requestEtag === cacheEtag) {
// make sure CDN cache header set here
this.setCDNHeaders(ctx);
// match etag, set status 304
ctx.status = 304;
return;
}
// get cache pkg data
const cacheBytes = await this.cacheService.getPackageManifests(fullname, isFullManifests);
if (cacheBytes && cacheBytes.length > 0) {
ctx.set('etag', `W/${cacheEtag}`);
ctx.type = 'json';
this.setCDNHeaders(ctx);
return cacheBytes;
}
}
} catch (e) {
this.logger.error(e);
this.logger.error('[ShowPackageController.show:error] get cache error, ignore');
}

// handle cache miss
Expand All @@ -78,7 +85,9 @@ export class ShowPackageController extends AbstractController {
// only set cache with normal request
// sync request response with no bug version fixed
if (!isSync) {
await this.cacheService.savePackageEtagAndManifests(fullname, isFullManifests, etag, cacheBytes);
ctx.runInBackground(async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

改成 eventBus 吧，别再使用 runInBackground 了

await this.cacheService.savePackageEtagAndManifests(fullname, isFullManifests, etag, cacheBytes);
});
}

// set etag
Expand Down
29 changes: 29 additions & 0 deletions test/port/controller/package/ShowPackageController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,35 @@ describe('test/port/controller/package/ShowPackageController.test.ts', () => {
assert.match(res.body.rev, /^\d+\-\w{24}$/);
});

describe('should fallback when cache error', async () => {
it('should fallback when get etag error', async () => {
app.mockLog();
mock(CacheService.prototype, 'getPackageEtag', async () => {
throw new Error('mock get etag error');
});
await app.httpRequest()
.get(`/${name}`)
.expect(200);

app.expectLog(/ShowPackageController.show:error/);
});

it('should fallback when get getPackageManifests error', async () => {
app.mockLog();
mock(CacheService.prototype, 'getPackageEtag', async () => {
return 'mock-etag';
});
mock(CacheService.prototype, 'getPackageManifests', async () => {
throw new Error('mock get etag error');
});
await app.httpRequest()
.get(`/${name}`)
.expect(200);

app.expectLog(/ShowPackageController.show:error/);
});
});

it('should show one package with full manifests', async () => {
mock(app.config.cnpmcore, 'syncMode', 'all');
const res = await app.httpRequest()
Expand Down