Skip to content

Commit 2efc7af

Browse files
ljones140ljones140
committed
Refactor: Extract another method and test with real data
1 parent 6d56d2b commit 2efc7af

File tree

2 files changed

+99
-142
lines changed

2 files changed

+99
-142
lines changed

‎componentDetection.test.ts‎

Lines changed: 84 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -69,161 +69,113 @@ describe("ComponentDetection.makePackageUrl", () => {
6969
});
7070
});
7171

72-
describe("ComponentDetection.addPackagesToManifests", () => {
72+
describe("ComponentDetection.processComponentsToManifests", () => {
7373
test("adds package as direct dependency when no top level referrers", () => {
74-
const manifests: any[] = [];
75-
76-
const testPackage = {
77-
id: "test-package",
78-
packageUrl: "pkg:npm/test-package@1.0.0",
79-
isDevelopmentDependency: false,
80-
topLevelReferrers: [], // Empty array = direct dependency
81-
locationsFoundAt: ["package.json"],
82-
containerDetailIds: [],
83-
containerLayerIds: [],
84-
packageID: () => "pkg:npm/test-package@1.0.0",
85-
packageURL: { toString: () => "pkg:npm/test-package@1.0.0" }
86-
};
87-
88-
ComponentDetection.addPackagesToManifests([testPackage] as any, manifests);
74+
const componentsFound = [
75+
{
76+
component: {
77+
name: "test-package",
78+
version: "1.0.0",
79+
packageUrl: {
80+
Scheme: "pkg",
81+
Type: "npm",
82+
Name: "test-package",
83+
Version: "1.0.0"
84+
},
85+
id: "test-package 1.0.0 - npm"
86+
},
87+
isDevelopmentDependency: false,
88+
topLevelReferrers: [], // Empty = direct dependency
89+
locationsFoundAt: ["package.json"]
90+
}
91+
];
92+
93+
const manifests = ComponentDetection.processComponentsToManifests(componentsFound);
8994

9095
expect(manifests).toHaveLength(1);
9196
expect(manifests[0].name).toBe("package.json");
92-
93-
// Test the actual manifest state instead of mock calls
9497
expect(manifests[0].directDependencies()).toHaveLength(1);
9598
expect(manifests[0].indirectDependencies()).toHaveLength(0);
9699
expect(manifests[0].countDependencies()).toBe(1);
97100
});
98101

99102
test("adds package as indirect dependency when has top level referrers", () => {
100-
const manifests: any[] = [];
101-
102-
const testPackage = {
103-
id: "test-package",
104-
packageUrl: "pkg:npm/test-package@1.0.0",
105-
isDevelopmentDependency: false,
106-
topLevelReferrers: [{ packageUrl: "pkg:npm/parent-package@1.0.0" }], // Has referrers = indirect
107-
locationsFoundAt: ["package.json"],
108-
containerDetailIds: [],
109-
containerLayerIds: [],
110-
packageID: () => "pkg:npm/test-package@1.0.0",
111-
packageURL: { toString: () => "pkg:npm/test-package@1.0.0" }
112-
};
113-
114-
ComponentDetection.addPackagesToManifests([testPackage] as any, manifests);
103+
const componentsFound = [
104+
{
105+
component: {
106+
name: "test-package",
107+
version: "1.0.0",
108+
packageUrl: {
109+
Scheme: "pkg",
110+
Type: "npm",
111+
Name: "test-package",
112+
Version: "1.0.0"
113+
},
114+
id: "test-package 1.0.0 - npm"
115+
},
116+
isDevelopmentDependency: false,
117+
topLevelReferrers: [
118+
{
119+
name: "parent-package",
120+
version: "1.0.0",
121+
packageUrl: {
122+
Scheme: "pkg",
123+
Type: "npm",
124+
Name: "parent-package",
125+
Version: "1.0.0"
126+
}
127+
}
128+
],
129+
locationsFoundAt: ["package.json"]
130+
}
131+
];
132+
133+
const manifests = ComponentDetection.processComponentsToManifests(componentsFound);
115134

116135
expect(manifests).toHaveLength(1);
117136
expect(manifests[0].name).toBe("package.json");
118-
119-
// Test the actual manifest state - should be indirect dependency
120137
expect(manifests[0].directDependencies()).toHaveLength(0);
121138
expect(manifests[0].indirectDependencies()).toHaveLength(1);
122139
expect(manifests[0].countDependencies()).toBe(1);
123140
});
124141

125-
// Component detection reports some packages as top level referrers of themselves
126-
// We need to mark as direct as causes Dependency Graph to mark the package as transitive without any Direct
127-
test("adds package as indirect dependency when top level referrer is itself", () => {
128-
const manifests: any[] = [];
129-
130-
const testPackage = {
131-
id: "test-package",
132-
packageUrl: "pkg:npm/test-package@1.0.0",
133-
isDevelopmentDependency: false,
134-
topLevelReferrers: [{ packageUrl: "pkg:npm/test-package@1.0.0" }],
135-
locationsFoundAt: ["package.json"],
136-
containerDetailIds: [],
137-
containerLayerIds: [],
138-
packageID: () => "pkg:npm/test-package@1.0.0",
139-
packageURL: { toString: () => "pkg:npm/test-package@1.0.0" }
140-
};
141-
142-
ComponentDetection.addPackagesToManifests([testPackage] as any, manifests);
142+
test("adds package as direct dependency when top level referrer is itself", () => {
143+
const componentsFound = [
144+
{
145+
component: {
146+
name: "test-package",
147+
version: "1.0.0",
148+
packageUrl: {
149+
Scheme: "pkg",
150+
Type: "npm",
151+
Name: "test-package",
152+
Version: "1.0.0"
153+
},
154+
id: "test-package 1.0.0 - npm"
155+
},
156+
isDevelopmentDependency: false,
157+
topLevelReferrers: [
158+
{
159+
name: "test-package",
160+
version: "1.0.0",
161+
packageUrl: {
162+
Scheme: "pkg",
163+
Type: "npm",
164+
Name: "test-package",
165+
Version: "1.0.0"
166+
}
167+
}
168+
],
169+
locationsFoundAt: ["package.json"]
170+
}
171+
];
172+
173+
const manifests = ComponentDetection.processComponentsToManifests(componentsFound);
143174

144175
expect(manifests).toHaveLength(1);
145176
expect(manifests[0].name).toBe("package.json");
146-
147177
expect(manifests[0].directDependencies()).toHaveLength(1);
148178
expect(manifests[0].indirectDependencies()).toHaveLength(0);
149179
expect(manifests[0].countDependencies()).toBe(1);
150180
});
151-
152-
test("handles multiple packages with mixed dependency types", () => {
153-
const manifests: any[] = [];
154-
155-
const directTestPackage = {
156-
id: "direct-package",
157-
packageUrl: "pkg:npm/direct-package@1.0.0",
158-
isDevelopmentDependency: false,
159-
topLevelReferrers: [], // Direct
160-
locationsFoundAt: ["package.json"],
161-
containerDetailIds: [],
162-
containerLayerIds: [],
163-
packageID: () => "pkg:npm/direct-package@1.0.0",
164-
packageURL: { toString: () => "pkg:npm/direct-package@1.0.0" }
165-
};
166-
167-
const indirectTestPackage = {
168-
id: "indirect-package",
169-
packageUrl: "pkg:npm/indirect-package@1.0.0",
170-
isDevelopmentDependency: false,
171-
topLevelReferrers: [{ packageUrl: "pkg:npm/parent@1.0.0" }], // Indirect
172-
locationsFoundAt: ["package.json"],
173-
containerDetailIds: [],
174-
containerLayerIds: [],
175-
packageID: () => "pkg:npm/indirect-package@1.0.0",
176-
packageURL: { toString: () => "pkg:npm/indirect-package@1.0.0" }
177-
};
178-
179-
ComponentDetection.addPackagesToManifests([directTestPackage, indirectTestPackage] as any, manifests);
180-
181-
expect(manifests).toHaveLength(1);
182-
expect(manifests[0].name).toBe("package.json");
183-
184-
expect(manifests[0].directDependencies()).toHaveLength(1);
185-
expect(manifests[0].indirectDependencies()).toHaveLength(1);
186-
expect(manifests[0].countDependencies()).toBe(2);
187-
});
188-
189-
test("creates separate manifests for different locations", () => {
190-
const manifests: any[] = [];
191-
192-
const packageJsonTestPackage = {
193-
id: "package-json-dep",
194-
packageUrl: "pkg:npm/package-json-dep@1.0.0",
195-
isDevelopmentDependency: false,
196-
topLevelReferrers: [],
197-
locationsFoundAt: ["package.json"],
198-
containerDetailIds: [],
199-
containerLayerIds: [],
200-
packageID: () => "pkg:npm/package-json-dep@1.0.0",
201-
packageURL: { toString: () => "pkg:npm/package-json-dep@1.0.0" }
202-
};
203-
204-
const csprojTestPackage = {
205-
id: "csproj-dep",
206-
packageUrl: "pkg:nuget/csproj-dep@1.0.0",
207-
isDevelopmentDependency: false,
208-
topLevelReferrers: [],
209-
locationsFoundAt: ["project.csproj"],
210-
containerDetailIds: [],
211-
containerLayerIds: [],
212-
packageID: () => "pkg:nuget/csproj-dep@1.0.0",
213-
packageURL: { toString: () => "pkg:nuget/csproj-dep@1.0.0" }
214-
};
215-
216-
ComponentDetection.addPackagesToManifests([packageJsonTestPackage, csprojTestPackage] as any, manifests);
217-
218-
expect(manifests).toHaveLength(2);
219-
220-
const packageJsonManifest = manifests.find(m => m.name === "package.json");
221-
const csprojManifest = manifests.find(m => m.name === "project.csproj");
222-
223-
expect(packageJsonManifest).toBeDefined();
224-
expect(csprojManifest).toBeDefined();
225-
226-
expect(packageJsonManifest.countDependencies()).toBe(1);
227-
expect(csprojManifest.countDependencies()).toBe(1);
228-
});
229181
});

‎componentDetection.ts‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,17 @@ export default class ComponentDetection {
6868

6969
public static async getManifestsFromResults(): Promise<Manifest[] | undefined> {
7070
core.info("Getting manifests from results");
71+
const results = await fs.readFileSync(this.outputPath, 'utf8');
72+
var json: any = JSON.parse(results);
73+
return this.processComponentsToManifests(json.componentsFound);
74+
}
75+
76+
public static processComponentsToManifests(componentsFound: any[]): Manifest[] {
7177
// Parse the result file and add the packages to the package cache
7278
const packageCache = new PackageCache();
7379
const packages: Array<ComponentDetectionPackage> = [];
7480

75-
const results = await fs.readFileSync(this.outputPath, 'utf8');
76-
77-
var json: any = JSON.parse(results);
78-
json.componentsFound.forEach(async (component: any) => {
81+
componentsFound.forEach(async (component: any) => {
7982
// Skip components without packageUrl
8083
if (!component.component.packageUrl) {
8184
core.debug(`Skipping component detected without packageUrl: ${JSON.stringify({
@@ -113,6 +116,7 @@ export default class ComponentDetection {
113116
}
114117

115118
const referrerUrl = ComponentDetection.makePackageUrl(referrer.packageUrl);
119+
referrer.packageUrlString = referrerUrl
116120

117121
// Skip if the generated packageUrl is empty
118122
if (!referrerUrl) {
@@ -140,19 +144,18 @@ export default class ComponentDetection {
140144
return manifests;
141145
}
142146

143-
public static addPackagesToManifests(packages: Array<ComponentDetectionPackage>, manifests: Array<Manifest>): void {
144-
packages.forEach(async (pkg: ComponentDetectionPackage) => {
145-
pkg.locationsFoundAt.forEach(async (location: any) => {
147+
private static addPackagesToManifests(packages: Array<ComponentDetectionPackage>, manifests: Array<Manifest>): void {
148+
packages.forEach((pkg: ComponentDetectionPackage) => {
149+
pkg.locationsFoundAt.forEach((location: any) => {
146150
if (!manifests.find((manifest: Manifest) => manifest.name == location)) {
147151
const manifest = new Manifest(location, location);
148152
manifests.push(manifest);
149153
}
150154

151155
// Filter out self-references from topLevelReferrers
152156
const nonSelfReferrers = pkg.topLevelReferrers.filter((referrer: any) => {
153-
if (!referrer.packageUrl) return false;
154-
const referrerUrl = ComponentDetection.makePackageUrl(referrer.packageUrl);
155-
return referrerUrl !== pkg.packageUrl;
157+
if (!referrer.packageUrlString) return false;
158+
return referrer.packageUrlString !== pkg.packageUrlString;
156159
});
157160

158161
if (nonSelfReferrers.length == 0) {
@@ -249,10 +252,12 @@ export default class ComponentDetection {
249252
}
250253

251254
class ComponentDetectionPackage extends Package {
255+
public packageUrlString: string;
252256

253257
constructor(packageUrl: string, public id: string, public isDevelopmentDependency: boolean, public topLevelReferrers: [],
254258
public locationsFoundAt: [], public containerDetailIds: [], public containerLayerIds: []) {
255259
super(packageUrl);
260+
this.packageUrlString = packageUrl;
256261
}
257262
}
258263

0 commit comments

Comments
 (0)