Skip to content

Commit

Permalink
Resolved infinite loop between two layering PDPs, added new class to …
Browse files Browse the repository at this point in the history
…make resolving this easier (#944)
  • Loading branch information
tjcouch-sil authored Jun 18, 2024
2 parents 483e3b0 + 9f3bfa5 commit da79f82
Show file tree
Hide file tree
Showing 12 changed files with 1,087 additions and 313 deletions.
2 changes: 1 addition & 1 deletion extensions/src/platform-scripture/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export async function activate(context: ExecutionActivationContext) {
papi.projectDataProviders.registerProjectDataProviderEngineFactory(
SCRIPTURE_EXTENDER_PDPF_ID,
SCRIPTURE_EXTENDER_PROJECT_INTERFACES,
new ScriptureExtenderProjectDataProviderEngineFactory(),
new ScriptureExtenderProjectDataProviderEngineFactory(SCRIPTURE_EXTENDER_PDPF_ID),
);

const includeProjectsCommandPromise = papi.commands.registerCommand(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
import {
IProjectDataProviderEngine,
IProjectDataProviderEngineFactory,
ProjectMetadataWithoutFactoryInfo,
} from '@papi/core';
import papi from '@papi/backend';
import { IProjectDataProviderEngine, IProjectDataProviderEngineFactory } from '@papi/core';
import papi, { LayeringProjectDataProviderEngineFactory } from '@papi/backend';
import { escapeStringRegexp } from 'platform-bible-utils';
import ScriptureExtenderProjectDataProviderEngine, {
SCRIPTURE_EXTENDER_OVERLAY_PROJECT_INTERFACES,
Expand All @@ -13,7 +9,6 @@ import ScriptureExtenderProjectDataProviderEngine, {

/** PDP Factory ID for the Scripture Extender PDPF */
export const SCRIPTURE_EXTENDER_PDPF_ID = 'platformScripture.scriptureExtenderPdpf';
const SCRIPTURE_EXTENDER_PDPF_ID_REGEX_STRING = escapeStringRegexp(SCRIPTURE_EXTENDER_PDPF_ID);

/** Regex strings for the project interfaces the Scripture Extender Layering PDPF layers over */
const SCRIPTURE_EXTENDER_OVERLAY_PROJECT_INTERFACES_REGEX_STRINGS =
Expand All @@ -22,33 +17,12 @@ const SCRIPTURE_EXTENDER_OVERLAY_PROJECT_INTERFACES_REGEX_STRINGS =
);

class ScriptureExtenderProjectDataProviderEngineFactory
extends LayeringProjectDataProviderEngineFactory<typeof SCRIPTURE_EXTENDER_PROJECT_INTERFACES>
implements IProjectDataProviderEngineFactory<typeof SCRIPTURE_EXTENDER_PROJECT_INTERFACES>
{
// Implementing an interface method, so can't be static
// eslint-disable-next-line class-methods-use-this
async getAvailableProjects(): Promise<ProjectMetadataWithoutFactoryInfo[]> {
try {
const projectsToOverlayMetadata = await papi.projectLookup.getMetadataForAllProjects({
includeProjectInterfaces: SCRIPTURE_EXTENDER_OVERLAY_PROJECT_INTERFACES_REGEX_STRINGS,
// The `projectInterface`s we provide are excluded because we don't need to provide them
// again if they're already provided. Wrapped in another array so we only exclude projects
// that already serve all the `projectInterface`s we support
excludeProjectInterfaces: [SCRIPTURE_EXTENDER_PROJECT_INTERFACES],
excludePdpFactoryIds: SCRIPTURE_EXTENDER_PDPF_ID_REGEX_STRING,
});
return projectsToOverlayMetadata.map((projectMetadataToOverlay) => {
const projectMetadata: ProjectMetadataWithoutFactoryInfo & { pdpFactoryInfo?: unknown } =
projectMetadataToOverlay;
delete projectMetadata.pdpFactoryInfo;
projectMetadata.projectInterfaces = SCRIPTURE_EXTENDER_PROJECT_INTERFACES;
return projectMetadata;
});
} catch (e) {
throw new Error(
`${SCRIPTURE_EXTENDER_PDPF_ID} was not able to get metadata for projects to overlay. ${e}`,
);
}
}
projectInterfacesToLayerOver = SCRIPTURE_EXTENDER_OVERLAY_PROJECT_INTERFACES_REGEX_STRINGS;

providedProjectInterfaces = SCRIPTURE_EXTENDER_PROJECT_INTERFACES;

// Implementing an interface method, so can't be static
// eslint-disable-next-line class-methods-use-this
Expand Down
569 changes: 366 additions & 203 deletions lib/papi-dts/papi.d.ts

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions src/extension-host/services/papi-backend.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import dataProviderService, { DataProviderService } from '@shared/services/data-
import { DataProviderEngine as PapiDataProviderEngine } from '@shared/models/data-provider-engine.model';
import { ProjectDataProviderEngine as PapiProjectDataProviderEngine } from '@shared/models/project-data-provider-engine.model';
import { BaseProjectDataProviderEngine as PapiBaseProjectDataProviderEngine } from '@shared/models/base-project-data-provider-engine.model';
import { LayeringProjectDataProviderEngineFactory as PapiLayeringProjectDataProviderEngineFactory } from '@shared/models/project-data-provider-engine-factory.model';
import {
papiBackendProjectDataProviderService,
PapiBackendProjectDataProviderService,
Expand Down Expand Up @@ -54,6 +55,8 @@ const papi = {
ProjectDataProviderEngine: PapiProjectDataProviderEngine,
/** JSDOC DESTINATION BaseProjectDataProviderEngine */
BaseProjectDataProviderEngine: PapiBaseProjectDataProviderEngine,
/** JSDOC DESTINATION LayeringProjectDataProviderEngineFactory */
LayeringProjectDataProviderEngineFactory: PapiLayeringProjectDataProviderEngineFactory,

// Functions
/** This is just an alias for internet.fetch */
Expand Down Expand Up @@ -110,6 +113,9 @@ Object.freeze(papi.ProjectDataProviderEngine);
/** JSDOC DESTINATION BaseProjectDataProviderEngine */
export const { BaseProjectDataProviderEngine } = papi;
Object.freeze(papi.BaseProjectDataProviderEngine);
/** JSDOC DESTINATION LayeringProjectDataProviderEngineFactory */
export const { LayeringProjectDataProviderEngineFactory } = papi;
Object.freeze(papi.LayeringProjectDataProviderEngineFactory);
/** This is just an alias for internet.fetch */
export const { fetch } = papi;
Object.freeze(papi.fetch);
Expand Down
2 changes: 1 addition & 1 deletion src/renderer/components/dialogs/dialog-definition.model.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { DialogOptions } from '@shared/models/dialog-options.model';
import { DialogDefinitionBase, DialogProps } from '@renderer/components/dialogs/dialog-base.data';
import { ReactElement } from 'react';
import { ProjectMetadataFilterOptions } from '@shared/models/project-lookup.service-model';
import { ProjectMetadataFilterOptions } from '@shared/models/project-data-provider-factory.interface';

/** The tabType for the select project dialog in `select-project.dialog.tsx` */
export const SELECT_PROJECT_DIALOG_TYPE = 'platform.selectProject';
Expand Down
153 changes: 153 additions & 0 deletions src/shared/models/project-data-provider-engine-factory.model.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
import { ProjectInterfaces } from 'papi-shared-types';
import { ProjectMetadataFilterOptions } from '@shared/models/project-data-provider-factory.interface';
import { ProjectMetadataWithoutFactoryInfo } from '@shared/models/project-metadata.model';
import { IProjectDataProviderEngine } from '@shared/models/project-data-provider-engine.model';
import projectLookupService from '@shared/services/project-lookup.service';
import { escapeStringRegexp } from 'platform-bible-utils';

/**
* A factory object registered with the papi that creates a Project Data Provider Engine for each
* project with the factory's specified `projectInterface`s when the papi requests. Used by the papi
* to create {@link IProjectDataProviderEngine}s for a specific project and `projectInterface` when
* someone gets a project data provider with `papi.projectDataProviders.get`. When this factory
* object is registered with `papi.projectDataProviders.registerProjectDataProviderEngineFactory`,
* the papi creates a {@link ProjectDataProviderFactory} that layers over this engine to create
* {@link IProjectDataProvider}s.
*
* Project Data Provider Engine Factories create Project Data Provider Engines for specific
* `projectInterface`s. For each project id available on a Project Data Provider Factory, the
* factory that supports that project with some set of `projectInterface`s creates a new instance of
* a PDP with the supported `projectInterface`s.
*
* A PDP Factory can provide its own unique project ids (Base PDP Factory) or layer over other PDPFs
* and provide additional `projectInterface`s on those projects (Layering PDP Factory). Base PDP
* Factories must create PDPs that support the `platform.base` `projectInterface`. See
* {@link IBaseProjectDataProvider} and {@link ProjectDataProviderInterfaces} for more information.
*
* WARNING: For Layering PDPFs, `getAvailableProjects` has very specific requirements that
* **absolutely must** be met. Please see
* {@link IProjectDataProviderEngineFactory.getAvailableProjects} for more information on the
* requirements.
*
* To make creating a Layering PDPF easier, you can extend
* {@link LayeringProjectDataProviderEngineFactory}, which automatically fulfills the special
* requirements for Layering PDPFs. We highly recommend using it.
*/
export interface IProjectDataProviderEngineFactory<
SupportedProjectInterfaces extends ProjectInterfaces[],
> {
/** JSDOC DESTINATION IProjectDataProviderFactoryGetAvailableProjects */
getAvailableProjects(
layeringFilters?: ProjectMetadataFilterOptions,
): Promise<ProjectMetadataWithoutFactoryInfo[]>;
/**
* Create a {@link IProjectDataProviderEngine} for the project requested so the papi can create an
* {@link IProjectDataProvider} for the project. This project will have the same
* `projectInterface`s as this Project Data Provider Engine Factory
*
* @param projectId Id of the project for which to create a {@link IProjectDataProviderEngine}
* @returns A promise that resolves to a {@link IProjectDataProviderEngine} for the project passed
* in
*/
createProjectDataProviderEngine(
projectId: string,
): Promise<IProjectDataProviderEngine<SupportedProjectInterfaces>>;
}

/**
* JSDOC SOURCE LayeringProjectDataProviderEngineFactory
*
* Abstract class with partial implementation of {@link IProjectDataProviderEngineFactory}
* specifically for Layering PDPFs. You can extend this class to make creating a Layering PDPF
* easier.
*
* Extending this class automatically fulfills the special requirements for Layering PDPfs, so we
* highly recommend extending this class. Please see
* {@link IProjectDataProviderEngineFactory.getAvailableProjects} for more information on the
* requirements.
*/
export abstract class LayeringProjectDataProviderEngineFactory<
SupportedProjectInterfaces extends ProjectInterfaces[],
> {
/** Regex-escaped string of this `pdpfId`. */
protected pdpfIdRegexString: string;

/**
* String representation of `RegExp` pattern(s) to match against projects' `projectInterface`s
* (using the
* [`test`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/test)
* function) to determine if they should be included in the available projects this Layering PDPF
* provides. This is used as {@link ProjectMetadataFilterOptions.includeProjectInterfaces} when
* determining what available projects this layering PDPF supports. You should list here all
* combinations of `projectInterface`s your Layering PDPs require to make sure this layering PDPF
* announces that it supports the right projects.
*
* See {@link ProjectMetadataFilterOptions.includeProjectInterfaces} for more information on how to
* use this field.
*
* @example
*
* ```typescript
* projectInterfacesToLayerOver: ['one', ['two', 'three']];
* ```
*
* This layering PDPF will announce that it supports projects whose `projectInterface`s fulfill at
* least one of the following conditions (At least one entry in the array must match) and will
* provide `providedProjectInterfaces` for those projects:
*
* - Include `one`
* - Include both `two` and `three`.
*/
abstract projectInterfacesToLayerOver: undefined | string | (string | string[])[];
/**
* The list of `projectInterface`s that this layering PDPF provides on top of existing projects.
*
* @example
*
* ```typescript
* providedProjectInterfaces: ['four', 'five'];
* ```
*
* This layering PDPF will announce that its provides the `projectInterface`s `four` and `five`
* for projects that match `projectInterfacesToLayerOver`.
*/
abstract providedProjectInterfaces: SupportedProjectInterfaces;

/** @param pdpfId The id of this Project Data Provider Factory */
constructor(public pdpfId: string) {
this.pdpfIdRegexString = escapeStringRegexp(pdpfId);
}

/**
* Implementation of {@link IProjectDataProviderEngineFactory.getAvailableProjects} that properly
* fulfills the requirements of the method for Layering PDPFs. Announces that this Layering PDPF
* provides the `providedProjectInterfaces` for projects that match
* `projectInterfacesToLayerOver`.
*/
async getAvailableProjects(
layeringFilters?: ProjectMetadataFilterOptions,
): Promise<ProjectMetadataWithoutFactoryInfo[]> {
try {
const filters = projectLookupService.mergeMetadataFilters(layeringFilters, {
// We want to layer over certain `projectInterface`s with our interfaces, so
// include them in the list of projects we provide
includeProjectInterfaces: this.projectInterfacesToLayerOver,
// Make sure this pdpf is excluded so it doesn't get into an infinite loop with itself. This
// is also done when the project lookup service calls this method, so this is a sanity
// check more than anything else.
excludePdpFactoryIds: this.pdpfIdRegexString,
// Do not exclude the `projectInterface`s that this layering PDP serves. If we
// did exclude them because they already exist, we would cancel out with
// duplicate layering PDPs, and neither would serve these interfaces at all
});
const projectsToOverlayMetadata =
await projectLookupService.getMetadataForAllProjects(filters);
return projectsToOverlayMetadata.map((projectMetadataToOverlay) => {
projectMetadataToOverlay.projectInterfaces = this.providedProjectInterfaces;
return projectMetadataToOverlay;
});
} catch (e) {
throw new Error(`${this.pdpfId} was not able to get metadata for projects to overlay. ${e}`);
}
}
}
45 changes: 3 additions & 42 deletions src/shared/models/project-data-provider-engine.model.ts
Original file line number Diff line number Diff line change
@@ -1,49 +1,10 @@
import { ProjectInterfaces, ProjectInterfaceDataTypes } from 'papi-shared-types';
import IDataProviderEngine, { DataProviderEngine } from '@shared/models/data-provider-engine.model';
import { DataProviderDataTypes } from '@shared/models/data-provider.model';
import { ProjectMetadataWithoutFactoryInfo } from '@shared/models/project-metadata.model';
import { UnionToIntersection } from 'platform-bible-utils';

/**
* A factory object registered with the papi that creates a Project Data Provider Engine for each
* project with the factory's specified `projectInterface`s when the papi requests. Used by the papi
* to create {@link IProjectDataProviderEngine}s for a specific project and `projectInterface` when
* someone gets a project data provider with `papi.projectDataProviders.get`. When this factory
* object is registered with `papi.projectDataProviders.registerProjectDataProviderEngineFactory`,
* the papi creates a {@link ProjectDataProviderFactory} that layers over this engine to create
* {@link IProjectDataProvider}s.
*
* Project Data Provider Engine Factories create Project Data Provider Engines for specific
* `projectInterface`s. For each project id available on a Project Data Provider Factory, the
* factory that supports that project with some set of `projectInterface`s creates a new instance of
* a PDP with the supported `projectInterface`s.
*
* A PDP Factory can provide its own unique project ids (Base PDP Factory) or layer over other PDPFs
* and provide additional `projectInterface`s on those projects (Layering PDP Factory). Base PDP
* Factories must create PDPs that support the `platform.base` `projectInterface`. See
* {@link IBaseProjectDataProvider} and {@link ProjectDataProviderInterfaces} for more information.
*/
export interface IProjectDataProviderEngineFactory<
SupportedProjectInterfaces extends ProjectInterfaces[],
> {
/**
* Get a list of metadata objects for all projects that can be the targets of PDPs created by this
* factory engine
*/
getAvailableProjects(): Promise<ProjectMetadataWithoutFactoryInfo[]>;
/**
* Create a {@link IProjectDataProviderEngine} for the project requested so the papi can create an
* {@link IProjectDataProvider} for the project. This project will have the same
* `projectInterface`s as this Project Data Provider Engine Factory
*
* @param projectId Id of the project for which to create a {@link IProjectDataProviderEngine}
* @returns A promise that resolves to a {@link IProjectDataProviderEngine} for the project passed
* in
*/
createProjectDataProviderEngine(
projectId: string,
): Promise<IProjectDataProviderEngine<SupportedProjectInterfaces>>;
}
// Referenced in JSDoc below
// eslint-disable-next-line @typescript-eslint/no-unused-vars
import type { IProjectDataProviderEngineFactory } from '@shared/models/project-data-provider-engine-factory.model.ts';

/**
* The object to return from
Expand Down
40 changes: 37 additions & 3 deletions src/shared/models/project-data-provider-factory.interface.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import { Dispose } from 'platform-bible-utils';
import { Dispose, ModifierProject } from 'platform-bible-utils';
import { ProjectMetadataWithoutFactoryInfo } from './project-metadata.model';

export const PDP_FACTORY_OBJECT_TYPE: string = 'pdpFactory';

export type ProjectMetadataFilterOptions = ModifierProject & {
/** Project IDs to include */
includeProjectIds?: string | string[];
/** Project IDs to exclude */
excludeProjectIds?: string | string[];
};

/**
* Network object that creates Project Data Providers of a specific set of `projectInterface`s as
* requested on the `papi`. These are created internally within the platform to layer over
Expand All @@ -15,8 +22,35 @@ export const PDP_FACTORY_OBJECT_TYPE: string = 'pdpFactory';
* {@link IBaseProjectDataProvider} and {@link ProjectDataProviderInterfaces} for more information.
*/
interface IProjectDataProviderFactory extends Dispose {
/** Get data about all projects that can be created by this PDP factory */
getAvailableProjects(): Promise<ProjectMetadataWithoutFactoryInfo[]>;
/**
* JSDOC SOURCE IProjectDataProviderFactoryGetAvailableProjects
*
* Get metadata about all projects that can be served by PDPs created by this PDP factory.
*
* If this is a Base PDP Factory, this method should return this PDP Factory's own unique project
* IDs.
*
* If this is a Layering PDP Factory, this method should call
* `papi.projectLookup.getMetadataForAllProjects` with some set of metadata filters in order to
* determine which projects it can layer over. The set of metadata filters relevant to this PDP
* Factory **absolutely must** be merged with the `layeringFilters` provided using
* `papi.projectLookup.mergeMetadataFilters`, or it will get into an infinite loop of calling
* other layering PDPs.
*
* WARNING: If this is a Layering PDP Factory, it **absolutely must** merge its metadata filters
* with `layeringFilters` provided using `papi.projectLookup.mergeMetadataFilters`! Otherwise you
* will cause an infinite loop that will break things.
*
* @param layeringFilters If applicable, filters used to prevent this Layering PDP Factory from
* entering an infinite loop with another Layering PDP Factory. You **absolutely must** merge
* these filters with your own filters using `papi.projectLookup.mergeMetadataFilters` when
* calling `papi.projectLookup.getMetadataForAllProjects` inside this method. If you are not
* calling `getMetadataForAllProjects` inside this method (likely if this is a Base PDPF), you
* can safely ignore this parameter.
*/
getAvailableProjects(
layeringFilters?: ProjectMetadataFilterOptions,
): Promise<ProjectMetadataWithoutFactoryInfo[]>;
/**
* Returns the registered network object name of a PDP for the given project ID. Called by the
* platform when someone uses the project data provider service to access a project's data.
Expand Down
Loading

0 comments on commit da79f82

Please sign in to comment.