Skip to content

Commit

Permalink
Merge pull request #451 from winstonewert/master
Browse files Browse the repository at this point in the history
fix: resolve memory leak in CachedImage
  • Loading branch information
pmb0 authored Jun 21, 2021
2 parents e6a028e + f1a8849 commit 0b2a715
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 16 deletions.
8 changes: 4 additions & 4 deletions src/cached-image.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ describe('CachedImage', () => {
adapter = new ImageAdapterMock()
cache = new Map()

cachedImage = new CachedImage(new Keyv({ store: cache }), adapter)
cachedImage = new CachedImage(new Keyv({ store: cache }))
})

describe('fetch()', () => {
it('stores the image in the cache', async () => {
expect(await cachedImage.fetch('abc')).toBeUndefined()
expect(await cachedImage.fetch('abc', adapter)).toBeUndefined()

adapter.fetchMock = Buffer.from('foo')
expect((await cachedImage.fetch('def'))?.toString()).toBe('foo')
expect((await cachedImage.fetch('def', adapter))?.toString()).toBe('foo')

expect(cache).toMatchInlineSnapshot(`
Map {
Expand All @@ -41,7 +41,7 @@ describe('CachedImage', () => {
// @ts-ignore
await cachedImage.cache.set('image:foo', 'bar')

expect((await cachedImage.fetch('foo'))?.toString()).toBe('bar')
expect((await cachedImage.fetch('foo', adapter))?.toString()).toBe('bar')
})
})
})
11 changes: 5 additions & 6 deletions src/cached-image.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import Keyv from 'keyv'
import { ImageAdapter } from './interfaces'
import { getLogger } from './logger'
import { singleton } from 'tsyringe'

@singleton()
export class CachedImage {
log = getLogger('cached-image')

constructor(
private readonly cache: Keyv<Buffer>,
private readonly adapter: ImageAdapter,
) {}
constructor(private readonly cache: Keyv<Buffer>) {}

async fetch(id: string): Promise<Buffer | undefined> {
async fetch(id: string, adapter: ImageAdapter): Promise<Buffer | undefined> {
const cacheKey = `image:${id}`

let image = await this.cache.get(cacheKey)
Expand All @@ -20,7 +19,7 @@ export class CachedImage {
return image
}

image = await this.adapter.fetch(id)
image = await adapter.fetch(id)

if (image) {
this.log(`Caching original image ${id} ...`)
Expand Down
8 changes: 7 additions & 1 deletion src/transformer.service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { container } from 'tsyringe'
import { ImageAdapter } from './interfaces'
import { ObjectHash } from './object-hash.service'
import { Transformer } from './transformer.service'
import { CachedImage } from './cached-image'

class ImageMockAdapter implements ImageAdapter {
// eslint-disable-next-line @typescript-eslint/require-await
Expand All @@ -15,7 +16,12 @@ describe('Transformer', () => {
let transformer: Transformer

beforeEach(() => {
transformer = new Transformer(container.resolve(ObjectHash), new Keyv())
const cache = new Keyv()
transformer = new Transformer(
container.resolve(ObjectHash),
cache,
new CachedImage(cache),
)
})

describe('getCropDimensions()', () => {
Expand Down
7 changes: 2 additions & 5 deletions src/transformer.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export class Transformer {
constructor(
private readonly objectHasher: ObjectHash,
private readonly cache: Keyv<Result>,
private readonly cachedOriginalImage: CachedImage,
) {}

getCropDimensions(maxSize: number, width: number, height?: number): number[] {
Expand Down Expand Up @@ -59,11 +60,7 @@ export class Transformer {

this.log(`Resizing ${id} with options:`, JSON.stringify(options))

const cachedOriginalImage = new CachedImage(
this.cache as Keyv,
imageAdapter,
)
const originalImage = await cachedOriginalImage.fetch(id)
const originalImage = await this.cachedOriginalImage.fetch(id, imageAdapter)

if (!originalImage) {
return {
Expand Down

0 comments on commit 0b2a715

Please sign in to comment.