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

fix(library management): removed id and libraryId properties of installed libraries (resolves #203) #296

Merged
merged 1 commit into from
Dec 2, 2019
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
28 changes: 11 additions & 17 deletions examples/implementation/FileLibraryStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export default class FileLibraryStorage implements ILibraryStorage {
filename: string,
stream: Stream
): Promise<boolean> {
if (!(await this.getId(library))) {
if (!(await this.libraryExists(library))) {
throw new Error(
`Can't add file ${filename} to library ${LibraryName.toUberName(
library
Expand All @@ -62,7 +62,7 @@ export default class FileLibraryStorage implements ILibraryStorage {
* @returns {Promise<void>}
*/
public async clearLibraryFiles(library: ILibraryName): Promise<void> {
if (!(await this.getId(library))) {
if (!(await this.libraryExists(library))) {
throw new Error(
`Can't clear library ${LibraryName.toUberName(
library
Expand Down Expand Up @@ -110,19 +110,6 @@ export default class FileLibraryStorage implements ILibraryStorage {
);
}

/**
* Returns the id of an installed library.
* @param {ILibraryName} library The library to get the id for
* @returns {Promise<number>} the id or undefined if the library is not installed
*/
public async getId(library: ILibraryName): Promise<number> {
const libraryPath = this.getFullPath(library, 'library.json');
if (await fsExtra.pathExists(libraryPath)) {
return crc32(libraryPath);
}
return undefined;
}

/**
* Returns all installed libraries or the installed libraries that have the machine names in the arguments.
* @param {...string[]} machineNames (optional) only return libraries that have these machine names
Expand Down Expand Up @@ -195,14 +182,22 @@ export default class FileLibraryStorage implements ILibraryStorage {
this.getFullPath(library, 'library.json'),
libraryMetadata
);
library.id = await this.getId(library);
return library;
} catch (error) {
await fsExtra.remove(libPath);
throw error;
}
}

/**
* Checks if the library has been installed.
* @param name the library name
* @returns true if the library has been installed
*/
public async libraryExists(name: ILibraryName): Promise<boolean> {
return fsExtra.pathExists(this.getDirectoryPath(name));
}

/**
* Gets a list of all library files that exist for this library.
* @param {ILibraryName} library
Expand Down Expand Up @@ -257,7 +252,6 @@ export default class FileLibraryStorage implements ILibraryStorage {
libraryMetadata
);
const newLibrary = InstalledLibrary.fromMetadata(libraryMetadata);
newLibrary.id = await this.getId(newLibrary);
return newLibrary;
}

Expand Down
2 changes: 0 additions & 2 deletions src/ContentTypeInformationRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ export default class ContentTypeInformationRepository {
'icon.svg'
)
: undefined,
id: localLib.id,
installed: true,
isUpToDate: true,
localMajorVersion: localLib.majorVersion,
Expand Down Expand Up @@ -245,7 +244,6 @@ export default class ContentTypeInformationRepository {
hubLib.canInstall = this.canInstallLibrary(hubLib, user);
hubLib.isUpToDate = true;
} else {
hubLib.id = localLib.id;
hubLib.installed = true;
hubLib.restricted =
this.libraryIsRestricted(localLib) &&
Expand Down
3 changes: 0 additions & 3 deletions src/InstalledLibrary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export default class InstalledLibrary implements IInstalledLibrary {
this.majorVersion = majorVersion;
this.minorVersion = minorVersion;
this.patchVersion = patchVersion;
this.id = undefined;
this.title = undefined;
this.runnable = undefined;
this.restricted = restricted;
Expand All @@ -35,8 +34,6 @@ export default class InstalledLibrary implements IInstalledLibrary {
public embedTypes?: ('iframe' | 'div')[];
public fullscreen?: 0 | 1;
public h?: number;
public id: number;
public libraryId: number;
public license?: string;
public metadataSettings?: { disable: 0 | 1; disableExtraTitleField: 0 | 1 };
public preloadedCss?: IPath[];
Expand Down
30 changes: 4 additions & 26 deletions src/LibraryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,6 @@ export default class LibraryManager {
return this.libraryStorage.getFileStream(library, file);
}

/**
* Get id to an existing installed library.
* If version number is not specified, the newest version will be returned.
* @param {ILibraryName} library Note that patch version is ignored.
* @returns {Promise<number>} The id of the specified library or undefined (if not installed).
*/
public async getId(library: ILibraryName): Promise<number> {
log.debug(`getting id for library ${LibraryName.toUberName(library)}`);
return this.libraryStorage.getId(library);
}

/**
* Get a list of the currently installed libraries.
* @param {String[]?} machineNames (if supplied) only return results for the machines names in the list
Expand All @@ -89,7 +78,6 @@ export default class LibraryManager {
const installedLib = InstalledLibrary.fromName(libName);
const info = await this.loadLibrary(libName);
installedLib.patchVersion = info.patchVersion;
installedLib.id = info.libraryId;
installedLib.runnable = info.runnable;
installedLib.title = info.title;
return installedLib;
Expand Down Expand Up @@ -157,7 +145,7 @@ export default class LibraryManager {
const newLibraryMetadata: ILibraryMetadata = await fsExtra.readJSON(
`${directory}/library.json`
);
if (await this.getId(newLibraryMetadata)) {
if (await this.libraryExists(newLibraryMetadata)) {
// Check if library is already installed.
if (await this.isPatchedLibrary(newLibraryMetadata)) {
// Update the library if it is only a patch of an existing library
Expand Down Expand Up @@ -209,17 +197,7 @@ export default class LibraryManager {
* @returns true if the library has been installed
*/
public async libraryExists(library: LibraryName): Promise<boolean> {
const installed = await this.getInstalled([library.machineName]);
if (!installed || !installed[library.machineName]) {
return false;
}
return (
installed[library.machineName].find(
l =>
l.majorVersion === library.majorVersion &&
l.minorVersion === l.minorVersion
) !== undefined
);
return this.libraryStorage.libraryExists(library);
}

/**
Expand Down Expand Up @@ -349,7 +327,7 @@ export default class LibraryManager {
library,
'library.json'
);
libraryMetadata.libraryId = await this.getId(library);
libraryMetadata.libraryId = await this.libraryExists(library);
return libraryMetadata;
} catch (ignored) {
log.warn(
Expand Down Expand Up @@ -379,7 +357,7 @@ export default class LibraryManager {
* @returns {Promise<boolean>} true if the library is ok. Throws errors if not.
*/
private async checkConsistency(library: ILibraryName): Promise<boolean> {
if (!(await this.libraryStorage.getId(library))) {
if (!(await this.libraryExists(library))) {
log.error(
`Error in library ${LibraryName.toUberName(
library
Expand Down
25 changes: 7 additions & 18 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,13 +417,6 @@ export interface ILibraryStorage {
*/
getFileStream(library: ILibraryName, file: string): ReadStream;

/**
* Returns the id of an installed library.
* @param library The library to get the id for
* @returns the id or undefined if the library is not installed
*/
getId(library: ILibraryName): Promise<number>;

/**
* Returns all installed libraries or the installed libraries that have the machine names in the arguments.
* @param machineNames (optional) only return libraries that have these machine names
Expand Down Expand Up @@ -451,6 +444,13 @@ export interface ILibraryStorage {
restricted: boolean
): Promise<IInstalledLibrary>;

/**
* Checks if the library has been installed.
* @param name the library name
* @returns true if the library has been installed
*/
libraryExists(name: ILibraryName): Promise<boolean>;

/**
* Gets a list of all library files that exist for this library.
* @param library
Expand Down Expand Up @@ -626,17 +626,6 @@ export interface ISemanticsEntry {
* Objects of this interface represent installed libraries that have an id.
*/
export interface IInstalledLibrary extends ILibraryMetadata {
/**
* The id used internally to identify the library. Must be unique and assigned when the
* library is installed. Libraries whose machine name is identical but that have different
* major or minor version must also have different ids, while libraries that only differ in
* patch versions can have the same id (as they can't co-exist).
*/
id: number;
/**
* Unknown. Check if obsolete and to be removed.
*/
libraryId: number;
/**
* If set to true, the library can only be used be users who have this special
* privilege.
Expand Down