Fix bug with unnecessary plugin auto-update attempts

Summary: I've noticed that Flipper is trying to schedule auto-update for some plugins on every startup even though they are already updated. I found this happens because of a race condition when the auto-updater can be triggered before plugins initialised. This diff fixes that.

Reviewed By: passy

Differential Revision: D28312086

fbshipit-source-id: 66b0bd2aa9dfede1737d565b1e7e7845c940405e
This commit is contained in:
Anton Nikolaev
2021-05-11 17:02:24 -07:00
committed by Facebook GitHub Bot
parent 32f276b499
commit 0dce247741
5 changed files with 26 additions and 2 deletions

View File

@@ -62,6 +62,7 @@ Object {
"disabledPlugins": Array [],
"failedPlugins": Array [],
"gatekeepedPlugins": Array [],
"initialised": false,
"installedPlugins": Map {},
"loadedPlugins": Map {},
"marketplacePlugins": Array [],

View File

@@ -22,6 +22,7 @@ import {
registerBundledPlugins,
registerMarketplacePlugins,
MarketplacePluginDetails,
pluginsInitialised,
} from '../reducers/plugins';
import GK from '../fb-stubs/GK';
import {FlipperBasePlugin} from '../plugin';
@@ -106,6 +107,7 @@ export default async (store: Store, logger: Logger) => {
store.dispatch(addDisabledPlugins(disabledPlugins));
store.dispatch(addFailedPlugins(failedPlugins));
store.dispatch(registerPlugins(initialPlugins));
store.dispatch(pluginsInitialised());
sideEffect(
store,
@@ -165,7 +167,7 @@ function getBundledPlugins(): Array<BundledPluginDetails> {
try {
bundledPlugins = global.electronRequire(pluginPath);
} catch (e) {
console.error(e);
console.error('Failed to load bundled plugins', e);
}
return bundledPlugins;

View File

@@ -42,6 +42,7 @@ test('add clientPlugin', () => {
marketplacePlugins: [],
uninstalledPlugins: new Set(),
installedPlugins: new Map(),
initialised: false,
},
registerPlugins([testPlugin]),
);
@@ -62,6 +63,7 @@ test('add devicePlugin', () => {
marketplacePlugins: [],
uninstalledPlugins: new Set(),
installedPlugins: new Map(),
initialised: false,
},
registerPlugins([testDevicePlugin]),
);
@@ -82,6 +84,7 @@ test('do not add plugin twice', () => {
marketplacePlugins: [],
uninstalledPlugins: new Set(),
installedPlugins: new Map(),
initialised: false,
},
registerPlugins([testPlugin, testPlugin]),
);
@@ -118,6 +121,7 @@ test('add gatekeeped plugin', () => {
marketplacePlugins: [],
installedPlugins: new Map(),
uninstalledPlugins: new Set(),
initialised: false,
},
addGatekeepedPlugins(gatekeepedPlugins),
);

View File

@@ -39,6 +39,7 @@ export type State = {
marketplacePlugins: Array<MarketplacePluginDetails>;
uninstalledPlugins: Set<string>;
installedPlugins: Map<string, InstalledPluginDetails>;
initialised: boolean;
};
export type RegisterPluginAction = {
@@ -91,6 +92,9 @@ export type Action =
| {
type: 'PLUGIN_LOADED';
payload: PluginDefinition;
}
| {
type: 'PLUGINS_INITIALISED';
};
const INITIAL_STATE: State = {
@@ -105,6 +109,7 @@ const INITIAL_STATE: State = {
marketplacePlugins: [],
uninstalledPlugins: new Set(),
installedPlugins: new Map(),
initialised: false,
};
export default function reducer(
@@ -118,7 +123,6 @@ export default function reducer(
if (devicePlugins.has(p.id) || clientPlugins.has(p.id)) {
return;
}
if (isDevicePluginDefinition(p)) {
devicePlugins.set(p.id, p);
} else {
@@ -197,6 +201,10 @@ export default function reducer(
draft.uninstalledPlugins.delete(plugin.id);
draft.loadedPlugins.set(plugin.id, plugin.details);
});
} else if (action.type === 'PLUGINS_INITIALISED') {
return produce(state, (draft) => {
draft.initialised = true;
});
} else {
return state;
}
@@ -277,3 +285,7 @@ export const pluginLoaded = (payload: PluginDefinition): Action => ({
type: 'PLUGIN_LOADED',
payload,
});
export const pluginsInitialised = (): Action => ({
type: 'PLUGINS_INITIALISED',
});

View File

@@ -768,6 +768,7 @@ test('test determinePluginsToProcess for mutilple clients having plugins present
marketplacePlugins: [],
installedPlugins: new Map(),
uninstalledPlugins: new Set(),
initialised: true,
};
const op = determinePluginsToProcess(
[client1, client2, client3],
@@ -842,6 +843,7 @@ test('test determinePluginsToProcess for no selected plugin present in any clien
marketplacePlugins: [],
installedPlugins: new Map(),
uninstalledPlugins: new Set(),
initialised: true,
};
const op = determinePluginsToProcess([client1, client2], device1, plugins);
expect(op).toBeDefined();
@@ -893,6 +895,7 @@ test('test determinePluginsToProcess for multiple clients on same device', async
marketplacePlugins: [],
installedPlugins: new Map(),
uninstalledPlugins: new Set(),
initialised: true,
};
const op = determinePluginsToProcess([client1, client2], device1, plugins);
expect(op).toBeDefined();
@@ -982,6 +985,7 @@ test('test determinePluginsToProcess for multiple clients on different device',
marketplacePlugins: [],
installedPlugins: new Map(),
uninstalledPlugins: new Set(),
initialised: true,
};
const op = determinePluginsToProcess(
[client1Device1, client2Device1, client1Device2, client2Device2],
@@ -1067,6 +1071,7 @@ test('test determinePluginsToProcess to ignore archived clients', async () => {
marketplacePlugins: [],
installedPlugins: new Map(),
uninstalledPlugins: new Set(),
initialised: true,
};
const op = determinePluginsToProcess(
[client, archivedClient],