Skip to content

Commit

Permalink
fix: engines resolver ignores unbound semver ranges #50 (#64)
Browse files Browse the repository at this point in the history
  • Loading branch information
regevbr committed Apr 27, 2020
1 parent 24a515d commit 5d2f9ef
Show file tree
Hide file tree
Showing 36 changed files with 1,216 additions and 260 deletions.
3 changes: 2 additions & 1 deletion .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
"contributorsrc",
"simplegit",
"lcov",
"toolbelt"
"toolbelt",
"packument"
],
"ignorePaths": [
"**/coverage/**",
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,4 @@ typings/
/reports
!@types/**
!/tools

10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@
"format:fix": "yarn run prettier --write \"./**/*.{ts,js,json,md}\"",
"format:check": "yarn run prettier --list-different \"./**/*.{ts,js,json,md}\"",
"check:spelling": "cspell --config=.cspell.json \"**/*.{md,ts,js}\"",
"generate-contributors": "yarn run ts-node --transpile-only ./tools/generateContributors.ts && yarn run all-contributors generate",
"generate-migrations": "yarn run ts-node node_modules/.bin/typeorm migration:generate --config tools/getOrmConnection.ts --name autoGeneratedMigration",
"generate-contributors": "yarn run ts-node --transpile-only --project ./tsconfig.json ./tools/generateContributors.ts && yarn run all-contributors generate",
"generate-migrations": "yarn run ts-node --transpile-only --project ./tsconfig.json node_modules/.bin/typeorm migration:generate --config tools/getOrmConnection.ts --name autoGeneratedMigration",
"check-clean-workspace-after-install": "git diff --quiet --exit-code",
"docs:changelog": "yarn run changelog --allow-unknown",
"mutation-test": "yarn run stryker run"
Expand All @@ -87,7 +87,7 @@
"@types/cross-spawn": "^6.0.1",
"@types/jest": "^25.2.1",
"@types/jest-when": "^2.7.1",
"@types/node": "^13.13.2",
"@types/node": "^13.13.4",
"@types/node-fetch": "^2.5.7",
"@types/pacote": "^11.1.0",
"@types/semver": "^7.1.0",
Expand Down Expand Up @@ -117,7 +117,7 @@
"prettier": "^2.0.5",
"semver": "^7.3.2",
"ts-jest": "^25.4.0",
"ts-node": "^8.9.0",
"ts-node": "^8.9.1",
"ts-toolbelt": "^6.5.1",
"typescript": "^3.8.3"
},
Expand All @@ -135,7 +135,7 @@
"shell-quote": "~1.7.2",
"simple-git": "~1.132.0",
"sqlite3": "~4.1.1",
"tmp": "~0.1.0",
"tmp": "~0.2.0",
"ts-type-guards": "~0.6.1",
"typeorm": "~0.2.24",
"yaml": "~1.9.2",
Expand Down
24 changes: 16 additions & 8 deletions src/db/entities/dependency.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@
import { Entity, Column, PrimaryColumn } from 'typeorm';
import { IEntity } from '../interfaces/IEntity';

export interface IDependencyOptions {
export interface IDependencyOptionsBase {
name: string;
version: string;
targetNode: string;
match: boolean | undefined;
reason: string | undefined;
}

export interface IDependencyOptionsMatch extends IDependencyOptionsBase {
match: true;
reason: string;
}

export interface IDependencyOptionsNoMatch extends IDependencyOptionsBase {
match: false;
reason: null;
}

export type IDependencyOptions = IDependencyOptionsMatch | IDependencyOptionsNoMatch;

@Entity()
export class Dependency extends IEntity {
public static TAG = Symbol.for(`Dependency`);
Expand All @@ -22,15 +32,13 @@ export class Dependency extends IEntity {
@PrimaryColumn(`text`)
public targetNode!: string;

@Column(`boolean`, {
nullable: true,
})
public match!: boolean | undefined;
@Column(`boolean`)
public match!: boolean;

@Column(`text`, {
nullable: true,
})
public reason!: string | undefined;
public reason!: string | null;

constructor(options?: IDependencyOptions) {
super();
Expand Down
45 changes: 38 additions & 7 deletions src/db/entities/dependencyVersion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,13 @@ const dateFormat = `YYYY-MM-DD`;
export interface IDependencyVersionOptions {
name: string;
version: string;
repoUrl: string;
releaseDate: Moment;
commitSha: string;
repoUrl: string | null;
repoDirectory: string | null;
releaseDate: Moment | null;
commitSha: string | null;
engines: string | null;
testScript: string | null;
buildScript: string | null;
}

@Entity()
Expand All @@ -23,27 +27,54 @@ export class DependencyVersion extends IEntity {
@PrimaryColumn(`text`)
public version!: string;

@Column(`text`)
public repoUrl!: string;
@Column(`text`, {
nullable: true,
})
public repoUrl!: string | null;

@Column(`text`, {
nullable: true,
})
public repoDirectory!: string | null;

@Column(`text`, {
transformer: buildDateTransformer(dateFormat),
nullable: true,
})
public releaseDate!: Moment | null;

@Column(`text`, {
nullable: true,
})
public commitSha!: string | null;

@Column(`text`, {
nullable: true,
})
public testScript!: string | null;

@Column(`text`, {
nullable: true,
})
public releaseDate!: Moment;
public buildScript!: string | null;

@Column(`text`, {
nullable: true,
})
public commitSha!: string;
public engines!: string | null;

constructor(options?: IDependencyVersionOptions) {
super();
if (options) {
this.name = options.name;
this.version = options.version;
this.repoUrl = options.repoUrl;
this.repoDirectory = options.repoDirectory;
this.releaseDate = options.releaseDate;
this.commitSha = options.commitSha;
this.engines = options.engines;
this.testScript = options.testScript;
this.buildScript = options.buildScript;
}
}
}
10 changes: 8 additions & 2 deletions src/db/impl/dateTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,16 @@ import moment = require('moment');

export const buildDateTransformer = (dateFormat: string): ValueTransformer => {
return {
to: (value: Moment): string => {
to: (value: Moment | null): string | null => {
if (!value) {
return null;
}
return value.format(dateFormat);
},
from: (value: string): Moment => {
from: (value: string | null): Moment | null => {
if (!value) {
return null;
}
return moment.utc(value, dateFormat);
},
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { MigrationInterface, QueryRunner } from 'typeorm';

// eslint-disable-next-line @typescript-eslint/class-name-casing
export class autoGeneratedMigration1587733162051 implements MigrationInterface {
name = `autoGeneratedMigration1587733162051`;
export class autoGeneratedMigration1587992024382 implements MigrationInterface {
name = `autoGeneratedMigration1587992024382`;

public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`CREATE TABLE "dependency" ("name" text NOT NULL, "version" text NOT NULL, "targetNode" text NOT NULL, "match" boolean, "reason" text, PRIMARY KEY ("name", "version", "targetNode"))`,
`CREATE TABLE "dependency" ("name" text NOT NULL, "version" text NOT NULL, "targetNode" text NOT NULL, "match" boolean NOT NULL, "reason" text, PRIMARY KEY ("name", "version", "targetNode"))`,
undefined
);
await queryRunner.query(
`CREATE TABLE "dependency_version" ("name" text NOT NULL, "version" text NOT NULL, "repoUrl" text NOT NULL, "releaseDate" text NOT NULL, "commitSha" text, PRIMARY KEY ("name", "version"))`,
`CREATE TABLE "dependency_version" ("name" text NOT NULL, "version" text NOT NULL, "repoUrl" text, "repoDirectory" text, "releaseDate" text, "commitSha" text, "testScript" text, "buildScript" text, "engines" text, PRIMARY KEY ("name", "version"))`,
undefined
);
}
Expand Down
55 changes: 38 additions & 17 deletions src/dependencyChecker/impl/dependencyChecker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import { ICacheResolver } from '../../resolvers/cacheResolver';
import { ICIResolver } from '../../resolvers/ciResolver';
import { ITestResolver } from '../../resolvers/testResolver';
import { IEnginesResolver } from '../../resolvers/enginesResolver';
import { IPackageInfo } from '../../utils/packageInfo';
import { ILoggerFactory } from '../../utils/logger';
import { ILogger } from '../../utils/logger/interfaces/ILogger';
import { IPackageInfoCache } from '..';

@injectable()
export class DependencyChecker extends IDependencyChecker {
Expand All @@ -23,31 +23,34 @@ export class DependencyChecker extends IDependencyChecker {
private readonly ciResolver: ICIResolver,
private readonly testResolver: ITestResolver,
private readonly engineResolver: IEnginesResolver,
private readonly packageInfo: IPackageInfo,
private readonly packageInfoCache: IPackageInfoCache,
loggerFactory: ILoggerFactory
) {
super();
this.logger = loggerFactory.getLogger(`Dependency Checker`);
}

public async run({
repo,
targetNode,
pkg,
workDir,
skip,
}: IDependencyCheckerRunOptions): Promise<IDependencyCheckerRunResult> {
this.logger.info(`Checking ${pkg.dependencyType} dependency - ${pkg.name}@${pkg.version}`);
this.logger.info(`Checking ${pkg.dependencyType} dependency - ${pkg.name}@${pkg.semver}`);
const dependencyVersion = await this.packageInfoCache.getPackageInfo({
semver: pkg.semver,
name: pkg.name,
});
if (!skip.cache.ignoreAll) {
const cacheResult = await this.cacheResolver.resolve({
targetNode,
repo: {
name: pkg.name,
version: pkg.version,
version: dependencyVersion.version,
},
});
if (cacheResult.isMatch) {
if (cacheResult.result && !skip.cache.ignoreTruthy) {
if (cacheResult.result) {
return {
isMatch: true,
resolverName: cacheResult.resolverName,
Expand All @@ -59,31 +62,49 @@ export class DependencyChecker extends IDependencyChecker {
}
}
}
// TODO get packageInfo from cache, probably we should move all this logic to a different unit
const packageInfo = await this.packageInfo.getPackageInfo({
semver: pkg.version,
name: pkg.name,
});
const engineResult = await this.engineResolver.resolve({
targetNode,
engines: packageInfo.engines,
engines: dependencyVersion.engines || undefined,
releaseDate: dependencyVersion.releaseDate || undefined,
});
if (engineResult.isMatch) {
return {
isMatch: true,
resolverName: engineResult.resolverName,
};
}
const repoPath = await this.gitCheckout.checkoutRepo({
tag: pkg.version,
commitSha: repo.commitSha,
if (!dependencyVersion.repoUrl) {
const reason = `Failed to get repository url for ${pkg.dependencyType} dependency - ${pkg.name}@${dependencyVersion.version}`;
this.logger.error(reason);
return {
isError: true,
reason,
};
}
if (!dependencyVersion.releaseDate) {
const reason = `Failed to get package release date for ${pkg.dependencyType} dependency - ${pkg.name}@${dependencyVersion.version}`;
this.logger.error(reason);
return {
isError: true,
reason,
};
}
const { repoPath, commitSha } = await this.gitCheckout.checkoutRepo({
tag: dependencyVersion.version,
commitSha: dependencyVersion.commitSha || undefined,
baseDir: workDir,
url: repo.url,
url: dependencyVersion.repoUrl,
});
if (!dependencyVersion.commitSha) {
await this.packageInfoCache.updateCommitSha({
dependencyVersion,
commitSha,
});
}
const ciResult = await this.ciResolver.resolve({
targetNode,
repoPath,
packageReleaseDate: pkg.releaseDate,
packageReleaseDate: dependencyVersion.releaseDate,
});
if (ciResult.isMatch) {
return {
Expand Down

0 comments on commit 5d2f9ef

Please sign in to comment.