Skip to content

Commit

Permalink
Combine related getBuffer* functions. Add tests (elastic#82766)
Browse files Browse the repository at this point in the history
## Summary
Move logic from `getBufferExtractorForContentType` into `getBufferExtractor` & change the interface so one function can be used.

### Diff showing old vs new call
```diff
-  getBufferExtractorForContentType(contentType);
+  getBufferExtractor({ contentType });
```
```diff
-  getBufferExtractor(archivePath);
+  getBufferExtractor({ archivePath });
```

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
  • Loading branch information
John Schulz committed Nov 5, 2020
1 parent 3a00dd3 commit 6b2fe07
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 31 deletions.
21 changes: 7 additions & 14 deletions x-pack/plugins/ingest_manager/server/services/epm/archive/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
import { PackageInvalidArchiveError, PackageUnsupportedMediaTypeError } from '../../../errors';
import { pkgToPkgKey } from '../registry';
import { cacheGet, cacheSet, setArchiveFilelist } from '../registry/cache';
import { unzipBuffer, untarBuffer, ArchiveEntry } from '../registry/extract';
import { ArchiveEntry, getBufferExtractor } from '../registry/extract';

export async function loadArchivePackage({
archiveBuffer,
Expand All @@ -37,24 +37,17 @@ export async function loadArchivePackage({
};
}

function getBufferExtractorForContentType(contentType: string) {
if (contentType === 'application/gzip') {
return untarBuffer;
} else if (contentType === 'application/zip') {
return unzipBuffer;
} else {
throw new PackageUnsupportedMediaTypeError(
`Unsupported media type ${contentType}. Please use 'application/gzip' or 'application/zip'`
);
}
}

export async function unpackArchiveToCache(
archiveBuffer: Buffer,
contentType: string,
filter = (entry: ArchiveEntry): boolean => true
): Promise<string[]> {
const bufferExtractor = getBufferExtractorForContentType(contentType);
const bufferExtractor = getBufferExtractor({ contentType });
if (!bufferExtractor) {
throw new PackageUnsupportedMediaTypeError(
`Unsupported media type ${contentType}. Please use 'application/gzip' or 'application/zip'`
);
}
const paths: string[] = [];
try {
await bufferExtractor(archiveBuffer, filter, (entry: ArchiveEntry) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export async function untarBuffer(
buffer: Buffer,
filter = (entry: ArchiveEntry): boolean => true,
onEntry = (entry: ArchiveEntry): void => {}
): Promise<void> {
): Promise<unknown> {
const deflatedStream = bufferToStream(buffer);
// use tar.list vs .extract to avoid writing to disk
const inflateStream = tar.list().on('entry', (entry: tar.FileStat) => {
Expand All @@ -36,7 +36,7 @@ export async function unzipBuffer(
buffer: Buffer,
filter = (entry: ArchiveEntry): boolean => true,
onEntry = (entry: ArchiveEntry): void => {}
): Promise<void> {
): Promise<unknown> {
const zipfile = await yauzlFromBuffer(buffer, { lazyEntries: true });
zipfile.readEntry();
zipfile.on('entry', async (entry: yauzl.Entry) => {
Expand All @@ -50,6 +50,26 @@ export async function unzipBuffer(
return new Promise((resolve, reject) => zipfile.on('end', resolve).on('error', reject));
}

type BufferExtractor = typeof unzipBuffer | typeof untarBuffer;
export function getBufferExtractor(
args: { contentType: string } | { archivePath: string }
): BufferExtractor | undefined {
if ('contentType' in args) {
if (args.contentType === 'application/gzip') {
return untarBuffer;
} else if (args.contentType === 'application/zip') {
return unzipBuffer;
}
} else if ('archivePath' in args) {
if (args.archivePath.endsWith('.zip')) {
return unzipBuffer;
}
if (args.archivePath.endsWith('.gz')) {
return untarBuffer;
}
}
}

function yauzlFromBuffer(buffer: Buffer, opts: yauzl.Options): Promise<yauzl.ZipFile> {
return new Promise((resolve, reject) =>
yauzl.fromBuffer(buffer, opts, (err?: Error, handle?: yauzl.ZipFile) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,38 @@ describe('splitPkgKey tests', () => {
});
});

describe('getBufferExtractor', () => {
it('returns unzipBuffer if the archive key ends in .zip', () => {
const extractor = getBufferExtractor('.zip');
describe('getBufferExtractor called with { archivePath }', () => {
it('returns unzipBuffer if `archivePath` ends in .zip', () => {
const extractor = getBufferExtractor({ archivePath: '.zip' });
expect(extractor).toBe(unzipBuffer);
});

it('returns untarBuffer if the key ends in anything else', () => {
const extractor = getBufferExtractor('.xyz');
it('returns untarBuffer if `archivePath` ends in .gz', () => {
const extractor = getBufferExtractor({ archivePath: '.gz' });
expect(extractor).toBe(untarBuffer);
const extractor2 = getBufferExtractor({ archivePath: '.tar.gz' });
expect(extractor2).toBe(untarBuffer);
});

it('returns `undefined` if `archivePath` ends in anything else', () => {
const extractor = getBufferExtractor({ archivePath: '.xyz' });
expect(extractor).toEqual(undefined);
});
});

describe('getBufferExtractor called with { contentType }', () => {
it('returns unzipBuffer if `contentType` is `application/zip`', () => {
const extractor = getBufferExtractor({ contentType: 'application/zip' });
expect(extractor).toBe(unzipBuffer);
});

it('returns untarBuffer if `contentType` is `application/gzip`', () => {
const extractor = getBufferExtractor({ contentType: 'application/gzip' });
expect(extractor).toBe(untarBuffer);
});

it('returns `undefined` if `contentType` ends in anything else', () => {
const extractor = getBufferExtractor({ contentType: '.xyz' });
expect(extractor).toEqual(undefined);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ import {
setArchiveFilelist,
deleteArchiveFilelist,
} from './cache';
import { ArchiveEntry, untarBuffer, unzipBuffer } from './extract';
import { ArchiveEntry, getBufferExtractor } from './extract';
import { fetchUrl, getResponse, getResponseStream } from './requests';
import { streamToBuffer } from './streams';
import { getRegistryUrl } from './registry_url';
import { appContextService } from '../..';
import { PackageNotFoundError, PackageCacheError } from '../../../errors';

export { ArchiveEntry } from './extract';
export { ArchiveEntry, getBufferExtractor } from './extract';

export interface SearchParams {
category?: CategoryId;
Expand Down Expand Up @@ -139,7 +139,10 @@ export async function unpackRegistryPackageToCache(
): Promise<string[]> {
const paths: string[] = [];
const { archiveBuffer, archivePath } = await fetchArchiveBuffer(pkgName, pkgVersion);
const bufferExtractor = getBufferExtractor(archivePath);
const bufferExtractor = getBufferExtractor({ archivePath });
if (!bufferExtractor) {
throw new Error('Unknown compression format. Please use .zip or .gz');
}
await bufferExtractor(archiveBuffer, filter, (entry: ArchiveEntry) => {
const { path, buffer } = entry;
const { file } = pathParts(path);
Expand Down Expand Up @@ -199,13 +202,6 @@ export function pathParts(path: string): AssetParts {
} as AssetParts;
}

export function getBufferExtractor(archivePath: string) {
const isZip = archivePath.endsWith('.zip');
const bufferExtractor = isZip ? unzipBuffer : untarBuffer;

return bufferExtractor;
}

export async function ensureCachedArchiveInfo(
name: string,
version: string,
Expand Down

0 comments on commit 6b2fe07

Please sign in to comment.