Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add versioning to sources.json #1069

Merged
merged 6 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15,074 changes: 6,920 additions & 8,154 deletions package-lock.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
},
"devDependencies": {
"@electron/notarize": "^2.2.0",
"@nordicsemiconductor/pc-nrfconnect-shared": "^191.0.0",
"@nordicsemiconductor/pc-nrfconnect-shared": "^195.0.0",
"@playwright/test": "^1.16.3",
"@testing-library/user-event": "^14.4.3",
"@types/chmodr": "1.0.0",
Expand Down
7 changes: 1 addition & 6 deletions src/launcher/features/settings/Settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,7 @@ const Sources = () => {
variant="outline-secondary"
size="sm"
onClick={() =>
clipboard.writeText(
source.url.replace(
/source\.json$/,
'apps.json'
)
)
clipboard.writeText(source.url)
}
title="Copy URL to clipboard"
>
Expand Down
2 changes: 1 addition & 1 deletion src/main/apps/apps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {
writeAppInfo,
} from './app';
import { downloadAppResources } from './appResource';
import { maybeMigrateLegacyMetaFiles } from './legacyMetaFiles';
import { maybeMigrateLegacyMetaFiles } from './dataMigration/legacyMetaFiles';
import {
downloadAllSources,
getAllAppUrls,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
createNewAppInfo,
} from './legacyMetaFiles';

jest.mock('../config', () => {
jest.mock('../../config', () => {
const { join } = jest.requireActual('path');
const { homedir } = jest.requireActual('os');
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ import {
import fs from 'fs-extra';
import path from 'path';

import { Source } from '../../ipc/sources';
import { getAppsRootDir, getNodeModulesDir } from '../config';
import { readFile, readJsonFile } from '../fileUtil';
import { installedAppPath, writeAppInfo } from './app';
import { Source } from '../../../ipc/sources';
import { getAppsRootDir, getNodeModulesDir } from '../../config';
import { readFile, readJsonFile } from '../../fileUtil';
import { installedAppPath, writeAppInfo } from '../app';
import {
sourceJsonExistsLocally,
writeSourceJson,
writeWithdrawnJson,
} from './sources';
} from '../sources';

type AppName = `pc-nrfconnect-${string}`;

Expand Down Expand Up @@ -96,7 +96,7 @@ export const createNewAppInfo = (
};
};

export const createNewAppInfoForWithdrawnApp = (
const createNewAppInfoForWithdrawnApp = (
source: Source,
appName: AppName,
packageJson: PackageJsonLegacyApp,
Expand Down
68 changes: 68 additions & 0 deletions src/main/apps/dataMigration/migrateSourcesJson.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright (c) 2023 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: LicenseRef-Nordic-4-Clause
*/

import { existsSync } from 'fs';

import { readSchemedJsonFile, writeSchemedJsonFile } from '../../fileUtil';
import { migrateSourcesJson, oldSourcesJsonPath } from './migrateSourcesJson';

jest.mock('fs');
jest.mock('../../fileUtil');

describe('migrating sources.json', () => {
const mockedExistsSync = jest.mocked(existsSync);
const mockedReadSchemedJsonFile = jest.mocked(readSchemedJsonFile);
const mockedWriteSchemedJsonFile = jest.mocked(writeSchemedJsonFile);

beforeEach(() => {
jest.resetAllMocks();
});

it('should migrate sources.json to sources-versioned.json', () => {
const oldSourceJsonContent = {
source1: 'http://source1.com/apps.json',
source2: 'http://source2.com/source.json',
};
const newSourceVersionedJsonContent = {
v1: [
{ name: 'source1', url: 'http://source1.com/source.json' },
{ name: 'source2', url: 'http://source2.com/source.json' },
],
};

// Arrange
mockedExistsSync.mockImplementation(
path => path === oldSourcesJsonPath()
);
mockedReadSchemedJsonFile.mockReturnValue(oldSourceJsonContent);

// Act
migrateSourcesJson();

// Assert
expect(mockedWriteSchemedJsonFile).toBeCalledWith(
expect.anything(),
expect.anything(),
newSourceVersionedJsonContent
);
});

it('should do nothing if sources.json does not exist', () => {
mockedExistsSync.mockReturnValue(false);

migrateSourcesJson();

expect(mockedReadSchemedJsonFile).not.toBeCalled();
});

it('should do nothing if sources-versioned.json already exists', () => {
mockedExistsSync.mockReturnValue(true);

migrateSourcesJson();

expect(mockedReadSchemedJsonFile).not.toBeCalled();
});
});
40 changes: 40 additions & 0 deletions src/main/apps/dataMigration/migrateSourcesJson.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright (c) 2015 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: LicenseRef-Nordic-4-Clause
*/

import fs from 'fs-extra';
import path from 'path';
import { z } from 'zod';

import { getAppsExternalDir } from '../../config';
import { readSchemedJsonFile } from '../../fileUtil';
import { sourcesVersionedJsonPath, writeSourcesFile } from '../sources';

export const oldSourcesJsonPath = () =>
path.join(getAppsExternalDir(), 'sources.json');

const oldSourcesJsonSchema = z.record(z.string(), z.string().url());

export const migrateSourcesJson = () => {
if (
!fs.existsSync(oldSourcesJsonPath()) ||
fs.existsSync(sourcesVersionedJsonPath())
) {
return;
}

const oldSourcesJson = readSchemedJsonFile(
oldSourcesJsonPath(),
oldSourcesJsonSchema
);
const migratedSources = Object.entries(oldSourcesJson).map(
([name, url]) => ({
name,
url: url.replace(/apps\.json$/, 'source.json'),
})
);

writeSourcesFile(migratedSources);
};
5 changes: 0 additions & 5 deletions src/main/apps/sourceChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ const downloadSource = async (
const sourceJson = await downloadSourceJson(url);
const source: Source = { name: sourceJson.name, url };

const isLegacyUrl = url.endsWith('/apps.json') && sourceJson.apps == null;
if (isLegacyUrl) {
return downloadSource(url.replace(/apps\.json$/, 'source.json'));
}

Comment on lines -37 to -41
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure I get the intention here, but why is this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost two years after we switched from using apps.json to source.json files, I consider this code to be unneeded and wanted to clean up by removing it.

The only case when it would still be used, would be if someone would add a URL like https://developer.nordicsemi.com/.pc-tools/nrfconnect-apps/test/apps.json (instead of …/source.json) in the dialog to add a source.

return { source, sourceJson };
};

Expand Down
3 changes: 3 additions & 0 deletions src/main/apps/sources.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

import { newWithdrawnJson } from './sources';

jest.mock('fs');
jest.mock('../fileUtil');

test('computing the new list of withdrawn apps', () => {
const oldWithdrawnJson = [
'http://example.org/oldWithdrawnApp.json',
Expand Down
83 changes: 43 additions & 40 deletions src/main/apps/sources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@

import {
SourceJson,
sourceJsonSchema,
WithdrawnJson,
withdrawnJsonSchema,
} from '@nordicsemiconductor/pc-nrfconnect-shared/main';
import { dialog } from 'electron';
import fs from 'fs-extra';
import path from 'path';
import { z } from 'zod';

import { SourceWithError } from '../../ipc/apps';
import { OFFICIAL, Source, SourceName, SourceUrl } from '../../ipc/sources';
Expand All @@ -21,7 +24,7 @@ import {
getNodeModulesDir,
} from '../config';
import describeError from '../describeError';
import { readFile, readJsonFile, writeJsonFile } from '../fileUtil';
import { readSchemedJsonFile, writeSchemedJsonFile } from '../fileUtil';
import { ensureDirExists } from '../mkdir';
import { downloadToJson } from '../net';

Expand All @@ -33,57 +36,48 @@ const officialSource = {
url: 'https://developer.nordicsemi.com/.pc-tools/nrfconnect-apps/source.json',
};

const sourcesJsonPath = () => path.join(getAppsExternalDir(), 'sources.json');
export const sourcesVersionedJsonPath = () =>
path.join(getAppsExternalDir(), 'sources-versioned.json');

const convertToOldSourceJsonFormat = (allSources: Source[]) =>
Object.fromEntries(
allSources.map(source => [
source.name,
source.url.replace(/source\.json$/, 'apps.json'),
])
);

const convertFromOldSourceJsonFormat = (
sourceJsonParsed: Record<SourceName, SourceUrl>
) =>
Object.entries(sourceJsonParsed).map(([name, url]) => ({
name,
url: url.replace(/apps\.json$/, 'source.json'),
}));
const sourcesVersionedSchema = z.object({
v1: z.array(
z.object({
name: z.string(),
url: z.string().url(),
})
),
});

const loadAllSources = () => {
if (!fs.existsSync(sourcesJsonPath())) {
if (!fs.existsSync(sourcesVersionedJsonPath())) {
return [];
}
let sourceJsonContent: string | undefined;
try {
sourceJsonContent = readFile(sourcesJsonPath());
const sourceJsonParsed = JSON.parse(sourceJsonContent);

if (Array.isArray(sourceJsonParsed)) {
return sourceJsonParsed;
}
if (sourceJsonParsed != null && typeof sourceJsonParsed === 'object') {
return convertFromOldSourceJsonFormat(sourceJsonParsed);
}

throw new Error('Unable to parse `source.json`.');
return readSchemedJsonFile(
sourcesVersionedJsonPath(),
sourcesVersionedSchema
).v1;
} catch (err) {
dialog.showErrorBox(
'Could not load list of locally known sources',
'No sources besides the official and the local one will be shown. ' +
'Also apps from other sources will be hidden.\n\nError: ' +
`${describeError(err)}\n\n` +
`Content of \`source.json\`+ \`${sourceJsonContent}\``
`${describeError(err)}`
);
return [];
}
return [];
};

export const writeSourcesFile = (allSources: Source[]) => {
writeSchemedJsonFile(sourcesVersionedJsonPath(), sourcesVersionedSchema, {
v1: allSources,
});
};

const saveAllSources = () => {
ensureSourcesAreLoaded();

writeJsonFile(sourcesJsonPath(), convertToOldSourceJsonFormat(sources));
writeSourcesFile(sources);
};

export const removeFromSourceList = (
Expand All @@ -108,7 +102,7 @@ export const addToSourceList = (sourceToAdd: Source, doSave = true) => {
}
};

export const ensureSourcesAreLoaded = () => {
const ensureSourcesAreLoaded = () => {
if (!sourcesAreLoaded) {
sourcesAreLoaded = true;

Expand Down Expand Up @@ -140,19 +134,23 @@ export const downloadSourceJson = (sourceUrl: SourceUrl) =>

const downloadSourceJsonToFile = async (source: Source) => {
const sourceJson = await downloadSourceJson(source.url);
writeJsonFile(getSourceJsonPath(source), sourceJson);
writeSourceJson(source, sourceJson);

return sourceJson;
};

const readSourceJson = (source: Source) =>
readJsonFile<SourceJson>(getSourceJsonPath(source), {
readSchemedJsonFile(getSourceJsonPath(source), sourceJsonSchema, {
name: source.name,
apps: [],
});

export const writeSourceJson = (source: Source, sourceJson: SourceJson) =>
writeJsonFile(getSourceJsonPath(source), sourceJson);
writeSchemedJsonFile(
getSourceJsonPath(source),
sourceJsonSchema,
sourceJson
);

export const getAppUrls = (source: Source) => readSourceJson(source).apps;

Expand All @@ -168,12 +166,17 @@ const getWithdrawnJsonPath = (source: Source) =>
path.join(getAppsRootDir(source.name), 'withdrawn.json');

const readWithdrawnJson = (source: Source) =>
readJsonFile<WithdrawnJson>(getWithdrawnJsonPath(source), []);
readSchemedJsonFile(getWithdrawnJsonPath(source), withdrawnJsonSchema, []);

export const writeWithdrawnJson = (
source: Source,
withdrawnJson: WithdrawnJson
) => writeJsonFile(getWithdrawnJsonPath(source), withdrawnJson);
) =>
writeSchemedJsonFile(
getWithdrawnJsonPath(source),
withdrawnJsonSchema,
withdrawnJson
);

export const isInListOfWithdrawnApps = (
source: SourceName,
Expand Down
Loading
Loading