Skip to content

Commit

Permalink
refactor: Restrict binaryName types (#387)
Browse files Browse the repository at this point in the history
> restrict binaryName type , the single source is the `config/binary.js`
file.

* export `BinaryName` & `CategoryName` type
* use `BinaryNameRule` typebox validator in controller
* `binaryName: string` => `binaryName: BinaryName`
  • Loading branch information
elrrrrrrr authored Feb 1, 2023
1 parent 1bcc169 commit 84eff97
Show file tree
Hide file tree
Showing 12 changed files with 79 additions and 50 deletions.
4 changes: 2 additions & 2 deletions app/common/adapter/binary/AbstractBinary.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ImplDecorator, Inject, QualifierImplDecoratorUtil } from '@eggjs/tegg';
import { BinaryType } from '../../enum/Binary';
import { EggHttpClient, EggLogger } from 'egg';
import { BinaryTaskConfig } from '../../../../config/binaries';
import { BinaryName, BinaryTaskConfig } from '../../../../config/binaries';

export type BinaryItem = {
name: string;
Expand All @@ -26,7 +26,7 @@ export abstract class AbstractBinary {
@Inject()
protected httpclient: EggHttpClient;

abstract fetch(dir: string, binaryName?: string): Promise<FetchResult | undefined>;
abstract fetch(dir: string, binaryName?: BinaryName): Promise<FetchResult | undefined>;

protected async requestXml(url: string) {
const { status, data, headers } = await this.httpclient.request(url, {
Expand Down
4 changes: 2 additions & 2 deletions app/common/adapter/binary/BucketBinary.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { SingletonProto } from '@eggjs/tegg';
import { BinaryType } from '../../enum/Binary';
import binaries, { BinaryTaskConfig } from 'config/binaries';
import binaries, { BinaryName, BinaryTaskConfig } from '../../../../config/binaries';
import path from 'path';
import { AbstractBinary, FetchResult, BinaryItem, BinaryAdapter } from './AbstractBinary';

@SingletonProto()
@BinaryAdapter(BinaryType.Bucket)
export class BucketBinary extends AbstractBinary {
async fetch(dir: string, binaryName: string): Promise<FetchResult | undefined> {
async fetch(dir: string, binaryName: BinaryName): Promise<FetchResult | undefined> {
// /foo/ => foo/
const binaryConfig = binaries[binaryName];
const subDir = dir.substring(1);
Expand Down
4 changes: 2 additions & 2 deletions app/common/adapter/binary/GithubBinary.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { SingletonProto } from '@eggjs/tegg';
import { BinaryType } from '../../enum/Binary';
import binaries, { BinaryTaskConfig } from '../../../../config/binaries';
import binaries, { BinaryName, BinaryTaskConfig } from '../../../../config/binaries';
import { AbstractBinary, FetchResult, BinaryItem, BinaryAdapter } from './AbstractBinary';

@SingletonProto()
Expand Down Expand Up @@ -72,7 +72,7 @@ export class GithubBinary extends AbstractBinary {
return items;
}

async fetch(dir: string, binaryName: string): Promise<FetchResult | undefined> {
async fetch(dir: string, binaryName: BinaryName): Promise<FetchResult | undefined> {
const binaryConfig = binaries[binaryName];
const releases = await this.initReleases(binaryConfig);
if (!releases) return;
Expand Down
4 changes: 2 additions & 2 deletions app/common/adapter/binary/ImageminBinary.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { SingletonProto } from '@eggjs/tegg';
import { BinaryType } from '../../enum/Binary';
import binaries from '../../../../config/binaries';
import binaries, { BinaryName } from '../../../../config/binaries';
import { AbstractBinary, FetchResult, BinaryItem, BinaryAdapter } from './AbstractBinary';

@SingletonProto()
@BinaryAdapter(BinaryType.Imagemin)
export class ImageminBinary extends AbstractBinary {

async fetch(dir: string, binaryName: string): Promise<FetchResult | undefined> {
async fetch(dir: string, binaryName: BinaryName): Promise<FetchResult | undefined> {
const binaryConfig = binaries[binaryName];
const dirItems: {
[key: string]: BinaryItem[];
Expand Down
4 changes: 2 additions & 2 deletions app/common/adapter/binary/NodeBinary.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { SingletonProto } from '@eggjs/tegg';
import { BinaryType } from '../../enum/Binary';
import binaries from '../../../../config/binaries';
import binaries, { BinaryName } from '../../../../config/binaries';
import { AbstractBinary, FetchResult, BinaryItem, BinaryAdapter } from './AbstractBinary';

@SingletonProto()
@BinaryAdapter(BinaryType.Node)
export class NodeBinary extends AbstractBinary {
async fetch(dir: string, binaryName: string): Promise<FetchResult | undefined> {
async fetch(dir: string, binaryName: BinaryName): Promise<FetchResult | undefined> {
const binaryConfig = binaries[binaryName];
const url = `${binaryConfig.distUrl}${dir}`;
const html = await this.requestXml(url);
Expand Down
4 changes: 2 additions & 2 deletions app/common/adapter/binary/NodePreGypBinary.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { SingletonProto } from '@eggjs/tegg';
import { BinaryType } from '../../enum/Binary';
import binaries from '../../../../config/binaries';
import binaries, { BinaryName } from '../../../../config/binaries';
import { join } from 'path';
import { AbstractBinary, FetchResult, BinaryItem, BinaryAdapter } from './AbstractBinary';

Expand All @@ -9,7 +9,7 @@ import { AbstractBinary, FetchResult, BinaryItem, BinaryAdapter } from './Abstra
export class NodePreGypBinary extends AbstractBinary {

// https://github.com/mapbox/node-pre-gyp
async fetch(dir: string, binaryName: string): Promise<FetchResult | undefined> {
async fetch(dir: string, binaryName: BinaryName): Promise<FetchResult | undefined> {
const binaryConfig = binaries[binaryName];
const pkgUrl = `https://registry.npmjs.com/${binaryName}`;
const data = await this.requestJSON(pkgUrl);
Expand Down
21 changes: 12 additions & 9 deletions app/core/service/BinarySyncerService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
EggHttpClient,
} from 'egg';
import fs from 'fs/promises';
import binaries from '../../../config/binaries';
import binaries, { BinaryName, CategoryName } from '../../../config/binaries';
import { NFSAdapter } from '../../common/adapter/NFSAdapter';
import { TaskType, TaskState } from '../../common/enum/Task';
import { downloadToTempfile } from '../../common/FileUtil';
Expand Down Expand Up @@ -43,15 +43,18 @@ export class BinarySyncerService extends AbstractService {
@Inject()
private readonly eggObjectFactory: EggObjectFactory;

public async findBinary(binaryName: string, parent: string, name: string) {
return await this.binaryRepository.findBinary(binaryName, parent, name);
// canvas/v2.6.1/canvas-v2.6.1-node-v57-linux-glibc-x64.tar.gz
// -> node-canvas-prebuilt/v2.6.1/node-canvas-prebuilt-v2.6.1-node-v57-linux-glibc-x64.tar.gz
// canvas 历史版本的 targetName 可能是 category 需要兼容
public async findBinary(targetName: BinaryName | CategoryName, parent: string, name: string) {
return await this.binaryRepository.findBinary(targetName, parent, name);
}

public async listDirBinaries(binary: Binary) {
return await this.binaryRepository.listBinaries(binary.category, `${binary.parent}${binary.name}`);
}

public async listRootBinaries(binaryName: string) {
public async listRootBinaries(binaryName: BinaryName) {
// 通常 binaryName 和 category 是一样的,但是有些特殊的 binaryName 会有多个 category,比如 canvas
// 所以查询 canvas 的时候,需要将 binaryName 和 category 的数据都查出来
const {
Expand Down Expand Up @@ -87,7 +90,7 @@ export class BinarySyncerService extends AbstractService {

// SyncBinary 由定时任务每台单机定时触发,手动去重
// 添加 bizId 在 db 防止重复,记录 id 错误
public async createTask(binaryName: string, lastData?: any) {
public async createTask(binaryName: BinaryName, lastData?: any) {
const existsTask = await this.taskRepository.findTaskByTargetName(binaryName, TaskType.SyncBinary);
if (existsTask) {
return existsTask;
Expand All @@ -112,7 +115,7 @@ export class BinarySyncerService extends AbstractService {
}

public async executeTask(task: Task) {
const binaryName = task.targetName;
const binaryName = task.targetName as BinaryName;
const binaryAdapter = await this.getBinaryAdapter(binaryName);
const logUrl = `${this.config.cnpmcore.registry}/-/binary/${binaryName}/syncs/${task.taskId}/log`;
let logs: string[] = [];
Expand Down Expand Up @@ -150,7 +153,7 @@ export class BinarySyncerService extends AbstractService {
}

private async syncDir(binaryAdapter: AbstractBinary, task: Task, dir: string, parentIndex = '') {
const binaryName = task.targetName;
const binaryName = task.targetName as BinaryName;
const result = await binaryAdapter.fetch(dir, binaryName);
let hasDownloadError = false;
let hasItems = false;
Expand Down Expand Up @@ -217,7 +220,7 @@ export class BinarySyncerService extends AbstractService {
return [ hasDownloadError, hasItems ];
}

private async diff(binaryName: string, dir: string, fetchItems: BinaryItem[]) {
private async diff(binaryName: BinaryName, dir: string, fetchItems: BinaryItem[]) {
const existsItems = await this.binaryRepository.listBinaries(binaryName, dir);
const existsMap = new Map<string, Binary>();
for (const item of existsItems) {
Expand Down Expand Up @@ -265,7 +268,7 @@ export class BinarySyncerService extends AbstractService {
return binary;
}

private async getBinaryAdapter(binaryName: string): Promise<AbstractBinary | undefined> {
private async getBinaryAdapter(binaryName: BinaryName): Promise<AbstractBinary | undefined> {
const config = this.config.cnpmcore;
const binaryConfig = binaries[binaryName];

Expand Down
19 changes: 14 additions & 5 deletions app/port/controller/BinarySyncController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import { NotFoundError } from 'egg-errors';
import { AbstractController } from './AbstractController';
import { BinarySyncerService } from '../../core/service/BinarySyncerService';
import { Binary } from '../../core/entity/Binary';
import binaries from '../../../config/binaries';

import binaries, { BinaryName } from '../../../config/binaries';
import { BinaryNameRule } from '../typebox';
@HTTPController()
export class BinarySyncController extends AbstractController {
@Inject()
Expand Down Expand Up @@ -50,8 +50,11 @@ export class BinarySyncController extends AbstractController {
path: '/-/binary/:binaryName(@[^/]{1,220}\/[^/]{1,220}|[^@/]{1,220})/:subpath(.*)',
method: HTTPMethodEnum.GET,
})
async showBinary(@Context() ctx: EggContext, @HTTPParam() binaryName: string, @HTTPParam() subpath: string) {
if (!binaries[binaryName]) {
async showBinary(@Context() ctx: EggContext, @HTTPParam() binaryName: BinaryName, @HTTPParam() subpath: string) {
// check binaryName valid
try {
ctx.tValidate(BinaryNameRule, binaryName);
} catch (e) {
throw new NotFoundError(`Binary "${binaryName}" not found`);
}
subpath = subpath || '/';
Expand Down Expand Up @@ -100,7 +103,13 @@ export class BinarySyncController extends AbstractController {
path: '/-/binary/:binaryName(@[^/]{1,220}\/[^/]{1,220}|[^@/]{1,220})',
method: HTTPMethodEnum.GET,
})
async showBinaryIndex(@Context() ctx: EggContext, @HTTPParam() binaryName: string) {
async showBinaryIndex(@Context() ctx: EggContext, @HTTPParam() binaryName: BinaryName) {
// check binaryName valid
try {
ctx.tValidate(BinaryNameRule, binaryName);
} catch (e) {
throw new NotFoundError(`Binary "${binaryName}" not found`);
}
return await this.showBinary(ctx, binaryName, '/');
}

Expand Down
4 changes: 2 additions & 2 deletions app/port/schedule/CreateSyncBinaryTask.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { EggAppConfig } from 'egg';
import { IntervalParams, Schedule, ScheduleType } from '@eggjs/tegg/schedule';
import { Inject } from '@eggjs/tegg';
import { BinarySyncerService } from '../../core/service/BinarySyncerService';
import binaries from '../../../config/binaries';
import binaries, { BinaryName } from '../../../config/binaries';

@Schedule<IntervalParams>({
type: ScheduleType.WORKER,
Expand All @@ -28,7 +28,7 @@ export class CreateSyncBinaryTask {
// 默认只同步 binaryName 的二进制,即使有不一致的 category,会在同名的 binaryName 任务中同步
// 例如 canvas 只同步 binaryName 为 canvas 的二进制,不同步 category 为 node-canvas-prebuilt 的二进制
// node-canvas-prebuilt 的二进制会在 node-canvas-prebuilt 的任务中同步
await this.binarySyncerService.createTask(binaryName);
await this.binarySyncerService.createTask(binaryName as BinaryName);
}
}
}
14 changes: 14 additions & 0 deletions app/port/typebox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Type, Static } from '@sinclair/typebox';
import { RegistryType } from '../common/enum/Registry';
import semver from 'semver';
import { HookType } from '../common/enum/Hook';
import binaryConfig from '../../config/binaries';

export const Name = Type.String({
transform: [ 'trim' ],
Expand All @@ -27,6 +28,13 @@ export const HookName = Type.String({

export const HookTypeType = Type.Enum(HookType);

export const BinaryNameRule = Type.String({
format: 'binary-name',
transform: [ 'trim' ],
minLength: 1,
maxLength: 220,
});

export const Tag = Type.String({
format: 'semver-tag',
transform: [ 'trim' ],
Expand Down Expand Up @@ -107,6 +115,12 @@ export function patchAjv(ajv: any) {
return !semver.validRange(tag);
},
});
ajv.addFormat('binary-name', {
type: 'string',
validate: (binaryName: string) => {
return !!binaryConfig[binaryName];
},
});
}

export const QueryPageOptions = Type.Object({
Expand Down
42 changes: 22 additions & 20 deletions config/binaries.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,18 @@
import { BinaryType } from '../app/common/enum/Binary';

export type BinaryTaskConfig = {
category: string; // 默认 category 为 binaryName,但是有些 binary 会有不同的 category,比如 canvas,包含 canvas 和 node-canvas-prebuilt 两个
category: CategoryName; // 默认 category 为 binaryName,但是有些 binary 会有不同的 category,比如 canvas,包含 canvas 和 node-canvas-prebuilt 两个
description: string;
type: BinaryType;
repo: string;
distUrl: string;
ignoreDirs?: string[];
ignoreFiles?: string[];
ignoreDirs?: readonly string[];
ignoreFiles?: readonly string[];
options?: {
nodePlatforms?: string[],
nodeArchs?: {
[key: string]: string[],
},
nodePlatforms?: readonly string[],
nodeArchs?: Record<string, readonly string[]>,
// Imagemin binFiles
binFiles?: {
[key: string]: string[],
},
binFiles?: Record<string, readonly string[]>,
// default is 1
maxPage?: number;
// custom npm package name, for ImageminBinary
Expand All @@ -27,9 +23,7 @@ export type BinaryTaskConfig = {
disable?: boolean;
};

const binaries: {
[category: string]: BinaryTaskConfig;
} = {
const binaries = {
// NwjsBinary
nwjs: {
category: 'nwjs',
Expand Down Expand Up @@ -462,7 +456,7 @@ const binaries: {
osx: [],
linux: [ 'x64', 'x86' ],
freebsd: [ 'x64', 'x86' ],
win32: [ ],
win32: [],
},
binFiles: {
osx: [ 'pngcrush' ],
Expand All @@ -483,8 +477,8 @@ const binaries: {
nodePlatforms: [ 'macos', 'linux', 'win' ],
nodeArchs: {
macos: [],
linux: [ ],
win: [ ],
linux: [],
win: [],
},
binFiles: {
macos: [ 'gif2webp' ],
Expand All @@ -506,8 +500,8 @@ const binaries: {
nodePlatforms: [ 'macos', 'linux', 'win' ],
nodeArchs: {
macos: [],
linux: [ ],
win: [ ],
linux: [],
win: [],
},
binFiles: {
macos: [ 'guetzli' ],
Expand All @@ -528,7 +522,7 @@ const binaries: {
nodeArchs: {
osx: [],
linux: [],
win32: [ ],
win32: [],
},
binFiles: {
osx: [ 'advpng' ],
Expand Down Expand Up @@ -879,6 +873,14 @@ const binaries: {
},
},
},
} as const;

export type BinaryName = keyof typeof binaries;
export type CategoryName = typeof binaries[BinaryName]['category'];

const BinaryConfigMap: Record<BinaryName, BinaryTaskConfig> = {
...binaries,
};

export default binaries;

export default BinaryConfigMap;
5 changes: 3 additions & 2 deletions test/core/service/BinarySyncerService/createTask.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import assert from 'assert';
import { app } from 'egg-mock/bootstrap';
import { BinarySyncerService } from 'app/core/service/BinarySyncerService';
import { BinaryName } from 'config/binaries';

describe('test/core/service/BinarySyncerService/createTask.test.ts', () => {
let binarySyncerService: BinarySyncerService;
Expand All @@ -11,8 +12,8 @@ describe('test/core/service/BinarySyncerService/createTask.test.ts', () => {

describe('createTask()', () => {
it('should ignore duplicate binary task', async () => {
const task = await binarySyncerService.createTask('banana', {});
const newTask = await binarySyncerService.createTask('banana', {});
const task = await binarySyncerService.createTask('banana' as BinaryName, {});
const newTask = await binarySyncerService.createTask('banana' as BinaryName, {});
assert(task?.taskId === newTask?.taskId);
assert(task?.bizId === 'SyncBinary:banana');
});
Expand Down

0 comments on commit 84eff97

Please sign in to comment.