Skip to content

Commit cd57073

Browse files
fengmk2Copilot
andauthored
fix: auto sync _npmUser field (#912)
closes #910 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Publish flow exposes an isPrivate flag and includes publisher (_npmUser) information in both full and abbreviated manifests. * **Behavior** * Sync now detects and reconciles metadata differences (deprecated, funding, publisher) per-version and per-tag, preserving existing publisher info for public packages. * **Tests** * Added tests covering publisher propagation and metadata auto-sync across sync scenarios. * **Chores** * Bumped packument dependency. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: MK (fengmk2) <[email protected]> Co-authored-by: Copilot <[email protected]>
1 parent 461c66a commit cd57073

File tree

8 files changed

+524
-8
lines changed

8 files changed

+524
-8
lines changed

app/core/service/PackageManagerService.ts

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ export interface PublishPackageCmd {
8282
'content' | 'localFile'
8383
>;
8484
tags?: string[];
85+
/**
86+
* private package or not
87+
*/
8588
isPrivate: boolean;
8689
// only use on sync package
8790
publishTime?: Date;
@@ -175,12 +178,24 @@ export class PackageManagerService extends AbstractService {
175178
}
176179
}
177180

178-
// set _npmUser field to cmd.packageJson
179-
cmd.packageJson._npmUser = {
180-
// clean user scope prefix
181-
name: publisher.displayName,
182-
email: publisher.email,
183-
};
181+
// override _npmUser field if is private package or _npmUser field not exists
182+
// otherwise avoid overriding _npmUser field from sync package, e.g.: oidc publish package
183+
// "_npmUser": {
184+
// "name": "GitHub Actions",
185+
// "email": "[email protected]",
186+
// "trustedPublisher": {
187+
// "id": "github",
188+
// "oidcConfigId": "oidc:d0f693c3-ada3-4197-b34c-b7aaeb524f11"
189+
// }
190+
// }
191+
if (cmd.isPrivate || !cmd.packageJson._npmUser) {
192+
// set _npmUser field to cmd.packageJson
193+
cmd.packageJson._npmUser = {
194+
// clean user scope prefix
195+
name: publisher.displayName,
196+
email: publisher.email,
197+
};
198+
}
184199

185200
// add _registry_name field to cmd.packageJson
186201
const registry = await this.getSourceRegistry(pkg);
@@ -250,6 +265,7 @@ export class PackageManagerService extends AbstractService {
250265
// npminstall require publish time to show the recently update versions
251266
publish_time: cmd.packageJson.publish_time,
252267
_source_registry_name: cmd.packageJson._source_registry_name,
268+
_npmUser: cmd.packageJson._npmUser,
253269
} as AbbreviatedPackageJSONType);
254270
const abbreviatedDistBytes = Buffer.from(abbreviated);
255271
const abbreviatedDistIntegrity = await calculateIntegrity(abbreviatedDistBytes);
@@ -581,6 +597,28 @@ export class PackageManagerService extends AbstractService {
581597
await this._mergeManifestDist(pkgVersion.abbreviatedDist, mergeAbbreviated);
582598
}
583599

600+
public async savePackageVersionManifestWithBuilder(
601+
pkgVersion: PackageVersion,
602+
manifestBuilder: JSONBuilder,
603+
abbreviatedBuilder?: JSONBuilder,
604+
) {
605+
const manifestBytes = manifestBuilder.build();
606+
const manifestIntegrity = await calculateIntegrity(manifestBytes);
607+
pkgVersion.manifestDist.size = manifestBytes.length;
608+
pkgVersion.manifestDist.shasum = manifestIntegrity.shasum;
609+
pkgVersion.manifestDist.integrity = manifestIntegrity.integrity;
610+
await this.distRepository.saveDist(pkgVersion.manifestDist, manifestBytes);
611+
if (abbreviatedBuilder) {
612+
const abbreviatedBytes = abbreviatedBuilder.build();
613+
const abbreviatedIntegrity = await calculateIntegrity(abbreviatedBytes);
614+
pkgVersion.abbreviatedDist.size = abbreviatedBytes.length;
615+
pkgVersion.abbreviatedDist.shasum = abbreviatedIntegrity.shasum;
616+
pkgVersion.abbreviatedDist.integrity = abbreviatedIntegrity.integrity;
617+
await this.distRepository.saveDist(pkgVersion.abbreviatedDist, abbreviatedBytes);
618+
}
619+
await this.packageRepository.savePackageVersion(pkgVersion);
620+
}
621+
584622
/**
585623
* save package version readme
586624
*/
@@ -1052,6 +1090,7 @@ export class PackageManagerService extends AbstractService {
10521090
manifestDist.shasum = manifestIntegrity.shasum;
10531091
manifestDist.integrity = manifestIntegrity.integrity;
10541092
await this.distRepository.saveDist(manifestDist, manifestBytes);
1093+
// FIXME: should store new size, shasum and integrity to database
10551094
}
10561095

10571096
private async _updatePackageManifestsToDists(

app/core/service/PackageSyncerService.ts

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { PresetRegistryName, SyncDeleteMode } from '../../common/constants.ts';
1515
import { TaskState, TaskType } from '../../common/enum/Task.ts';
1616
import { downloadToTempfile } from '../../common/FileUtil.ts';
1717
import { detectInstallScript, getScopeAndName } from '../../common/PackageUtil.ts';
18+
import { DistRepository } from '../../repository/DistRepository.ts';
1819
import type {
1920
AuthorType,
2021
PackageJSONType,
@@ -82,6 +83,8 @@ export class PackageSyncerService extends AbstractService {
8283
private readonly registryManagerService: RegistryManagerService;
8384
@Inject()
8485
private readonly scopeManagerService: ScopeManagerService;
86+
@Inject()
87+
private readonly distRepository: DistRepository;
8588

8689
public async createTask(fullname: string, options?: SyncPackageTaskOptions) {
8790
const [scope, name] = getScopeAndName(fullname);
@@ -1465,6 +1468,71 @@ ${diff.addedVersions.length} added, ${diff.removedVersions.length} removed, calc
14651468
logs.push(`[${isoNow()}] 🟢 Removed version ${version} success`);
14661469
}
14671470
}
1471+
// #endregion
1472+
1473+
// #region check different meta data
1474+
// these fields will be changed after publish, so we need to check if they are different
1475+
const fieldsToCheck = [
1476+
'deprecated',
1477+
'funding',
1478+
// this field won't change, but this is a bug(#910) on cnpmcore, so we need to check if it is different
1479+
'_npmUser',
1480+
];
1481+
// for performance reason, we won't check all versions by default, only check those versions on dist-tags
1482+
for (const version of Object.values(distTags)) {
1483+
// ignore already synced versions
1484+
if (updateVersions.includes(version) || diff.removedVersions.includes(version)) {
1485+
continue;
1486+
}
1487+
const pkgVersion = await this.packageRepository.findPackageVersion(pkg.packageId, version);
1488+
if (!pkgVersion) {
1489+
logs.push(`[${isoNow()}] 🚧 version ${version} not exists in database, skip check different meta data`);
1490+
continue;
1491+
}
1492+
const manifestBuilder = await this.distRepository.findPackageVersionManifestJSONBuilder(pkg.packageId, version);
1493+
if (!manifestBuilder) {
1494+
logs.push(`[${isoNow()}] 🚧 version ${version} manifest not exists, skip check different meta data`);
1495+
continue;
1496+
}
1497+
const abbreviatedManifestBuilder = await this.distRepository.findPackageAbbreviatedManifestJSONBuilder(
1498+
pkg.packageId,
1499+
version,
1500+
);
1501+
if (!abbreviatedManifestBuilder) {
1502+
logs.push(`[${isoNow()}] 🚧 version ${version} abbreviated manifest not exists, may be a bug here`);
1503+
}
1504+
let hasDifferent = false;
1505+
const diffMeta: Record<string, unknown> = {};
1506+
for (const field of fieldsToCheck) {
1507+
const remoteValue = packument.getBufferIn(['versions', version, field])?.toString();
1508+
const localValue = manifestBuilder.getBufferIn([field])?.toString();
1509+
if (remoteValue !== localValue) {
1510+
const newValue = remoteValue ? JSON.parse(remoteValue) : undefined;
1511+
if (newValue === undefined) {
1512+
// delete
1513+
manifestBuilder.deleteIn([field]);
1514+
abbreviatedManifestBuilder?.deleteIn([field]);
1515+
diffMeta[field] = '$$delete$$';
1516+
} else {
1517+
// update
1518+
manifestBuilder.setIn([field], newValue);
1519+
abbreviatedManifestBuilder?.setIn([field], newValue);
1520+
diffMeta[field] = newValue;
1521+
}
1522+
hasDifferent = true;
1523+
}
1524+
}
1525+
if (hasDifferent) {
1526+
await this.packageManagerService.savePackageVersionManifestWithBuilder(
1527+
pkgVersion,
1528+
manifestBuilder,
1529+
abbreviatedManifestBuilder,
1530+
);
1531+
updateVersions.push(version);
1532+
logs.push(`[${isoNow()}] 🟢 Synced version ${version} success, different meta: ${JSON.stringify(diffMeta)}`);
1533+
}
1534+
}
1535+
// #endregion
14681536

14691537
logs.push(
14701538
`[${isoNow()}] 🟢 Synced updated ${updateVersions.length} versions, removed ${diff.removedVersions.length} versions`,
@@ -1477,7 +1545,6 @@ ${diff.addedVersions.length} added, ${diff.removedVersions.length} removed, calc
14771545
await this.packageManagerService.refreshPackageChangeVersionsToDists(pkg, updateVersions, diff.removedVersions);
14781546
logs.push(`[${isoNow()}] 🟢 Refresh use ${Date.now() - start}ms`);
14791547
}
1480-
// #endregion
14811548

14821549
// #region update tags
14831550
// "dist-tags": {

app/repository/DistRepository.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,47 @@ export class DistRepository {
2929
}
3030
}
3131

32+
async findPackageVersionManifestJSONBuilder(
33+
packageId: string,
34+
version: string,
35+
includeReadme = false,
36+
): Promise<JSONBuilder | undefined> {
37+
const packageVersion = await this.packageRepository.findPackageVersion(packageId, version);
38+
if (packageVersion) {
39+
// include readme
40+
if (includeReadme) {
41+
const [builder, readme] = await Promise.all([
42+
this.readDistBytesToJSONBuilder(packageVersion.manifestDist),
43+
this.readDistBytesToString(packageVersion.readmeDist),
44+
]);
45+
if (builder) {
46+
builder.setIn(['readme'], readme);
47+
}
48+
return builder;
49+
}
50+
51+
// only return manifest builder
52+
return await this.readDistBytesToJSONBuilder(packageVersion.manifestDist);
53+
}
54+
}
55+
3256
async findPackageAbbreviatedManifest(packageId: string, version: string): Promise<PackageJSONType | undefined> {
3357
const packageVersion = await this.packageRepository.findPackageVersion(packageId, version);
3458
if (packageVersion) {
3559
return await this.readDistBytesToJSON(packageVersion.abbreviatedDist);
3660
}
3761
}
3862

63+
async findPackageAbbreviatedManifestJSONBuilder(
64+
packageId: string,
65+
version: string,
66+
): Promise<JSONBuilder | undefined> {
67+
const packageVersion = await this.packageRepository.findPackageVersion(packageId, version);
68+
if (packageVersion) {
69+
return await this.readDistBytesToJSONBuilder(packageVersion.abbreviatedDist);
70+
}
71+
}
72+
3973
async readDistBytesToJSON<T>(dist: Dist) {
4074
const str = await this.readDistBytesToString(dist);
4175
if (str) {

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@
8484
"registry"
8585
],
8686
"dependencies": {
87-
"@cnpmjs/packument": "^1.4.0",
87+
"@cnpmjs/packument": "^1.5.0",
8888
"@eggjs/redis": "beta",
8989
"@eggjs/scripts": "beta",
9090
"@eggjs/tracer": "beta",

test/TestUtil.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ const __filename = fileURLToPath(import.meta.url);
2323
const __dirname = path.dirname(__filename);
2424

2525
interface PackageOptions {
26+
_npmUser?: {
27+
name: string;
28+
email: string;
29+
};
2630
name?: string;
2731
version?: string;
2832
versionObject?: object;
@@ -216,6 +220,10 @@ export class TestUtil {
216220
const firstVersion = Object.keys(versions)[0];
217221
const version = versions[firstVersion];
218222
let updateAttach = false;
223+
if (options._npmUser) {
224+
pkg._npmUser = options._npmUser;
225+
version._npmUser = options._npmUser;
226+
}
219227
if (options.name) {
220228
pkg.name = options.name;
221229
version.name = options.name;

0 commit comments

Comments
 (0)