Skip to content

Commit

Permalink
fix: Properly handle protected accessors from mixin methods (#401)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mykola-mokhnach
mykola-mokhnach authored Aug 4, 2024
1 parent 16d3e10 commit f47f6af
Show file tree
Hide file tree
Showing 10 changed files with 314 additions and 204 deletions.
86 changes: 43 additions & 43 deletions lib/mixins/connect.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@ import events from './events';
import { timing, util } from '@appium/support';
import { retryInterval, waitForCondition } from 'asyncbox';
import _ from 'lodash';
import {
setAppIdKey,
getAppDict,
getAppIdKey,
setPageIdKey,
getRcpClient,
getIsSafari,
getIncludeSafari,
getBundleId,
getAdditionalBundleIds,
} from './property-accessors';

const APP_CONNECT_TIMEOUT_MS = 0;
const APP_CONNECT_INTERVAL_MS = 100;
Expand Down Expand Up @@ -36,7 +47,7 @@ export async function setConnectionKey () {
*
* @this {RemoteDebugger}
* @param {number} [timeout=APP_CONNECT_TIMEOUT_MS]
* @returns {Promise<import('@appium/types').StringRecord>}
* @returns {Promise<import('../types').AppDict>}
*/
export async function connect (timeout = APP_CONNECT_TIMEOUT_MS) {
this.setup();
Expand Down Expand Up @@ -66,19 +77,19 @@ export async function connect (timeout = APP_CONNECT_TIMEOUT_MS) {
const timer = new timing.Timer().start();
this.log.debug(`Waiting up to ${timeout}ms for applications to be reported`);
try {
await waitForCondition(() => !_.isEmpty(this._appDict), {
await waitForCondition(() => !_.isEmpty(getAppDict(this)), {
waitMs: timeout,
intervalMs: APP_CONNECT_INTERVAL_MS,
});
this.log.debug(
`Retrieved ${util.pluralize('application', _.size(this._appDict), true)} ` +
`Retrieved ${util.pluralize('application', _.size(getAppDict(this)), true)} ` +
`within ${timer.getDuration().asMilliSeconds.toFixed(0)}ms`
);
} catch (err) {
this.log.debug(`Timed out waiting for applications to be reported`);
}
}
return this._appDict || {};
return _.cloneDeep(getAppDict(this));
} catch (err) {
this.log.error(`Error setting connection key: ${err.message}`);
await this.disconnect();
Expand All @@ -92,9 +103,7 @@ export async function connect (timeout = APP_CONNECT_TIMEOUT_MS) {
* @returns {Promise<void>}
*/
export async function disconnect () {
if (this._rpcClient) {
await this._rpcClient.disconnect();
}
await getRcpClient(this)?.disconnect();
this.emit(events.EVENT_DISCONNECT, true);
this.teardown();
}
Expand All @@ -115,23 +124,23 @@ export async function selectApp (currentUrl = null, maxTries = SELECT_APP_RETRIE
rpcClient.shouldCheckForTarget = false;
try {
const timer = new timing.Timer().start();
if (!this._appDict || _.isEmpty(this._appDict)) {
if (_.isEmpty(getAppDict(this))) {
this.log.debug('No applications currently connected.');
return [];
}

const { appIdKey } = await searchForApp.bind(this)(currentUrl, maxTries, ignoreAboutBlankUrl);
if (this._appIdKey !== appIdKey) {
this.log.debug(`Received altered app id, updating from '${this._appIdKey}' to '${appIdKey}'`);
this._appIdKey = appIdKey;
if (getAppIdKey(this) !== appIdKey) {
this.log.debug(`Received altered app id, updating from '${getAppIdKey(this)}' to '${appIdKey}'`);
setAppIdKey(this, appIdKey);
}
logApplicationDictionary.bind(this)();
// translate the dictionary into a useful form, and return to sender
this.log.debug(`Finally selecting app ${this._appIdKey}`);
this.log.debug(`Finally selecting app ${getAppIdKey(this)}`);

/** @type {import('../types').Page[]} */
const fullPageArray = [];
for (const [app, info] of _.toPairs(this._appDict)) {
for (const [app, info] of _.toPairs(getAppDict(this))) {
if (!_.isArray(info.pageArray) || !info.isActive) {
continue;
}
Expand Down Expand Up @@ -162,14 +171,15 @@ export async function selectApp (currentUrl = null, maxTries = SELECT_APP_RETRIE
* @returns {Promise<void>}
*/
export async function selectPage (appIdKey, pageIdKey, skipReadyCheck = false) {
this._appIdKey = _.startsWith(`${appIdKey}`, 'PID:') ? `${appIdKey}` : `PID:${appIdKey}`;
this._pageIdKey = pageIdKey;
const fullAppIdKey = _.startsWith(`${appIdKey}`, 'PID:') ? `${appIdKey}` : `PID:${appIdKey}`;
setAppIdKey(this, fullAppIdKey);
setPageIdKey(this, pageIdKey);

this.log.debug(`Selecting page '${pageIdKey}' on app '${this._appIdKey}' and forwarding socket setup`);
this.log.debug(`Selecting page '${pageIdKey}' on app '${fullAppIdKey}' and forwarding socket setup`);

const timer = new timing.Timer().start();

await this.requireRpcClient().selectPage(this._appIdKey, pageIdKey);
await this.requireRpcClient().selectPage(fullAppIdKey, pageIdKey);

if (!skipReadyCheck && !await this.checkPageIsReady()) {
await this.waitForDom();
Expand All @@ -178,7 +188,6 @@ export async function selectPage (appIdKey, pageIdKey, skipReadyCheck = false) {
this.log.debug(`Selected page after ${timer.getDuration().asMilliSeconds.toFixed(0)}ms`);
}


/**
*
* @this {RemoteDebugger}
Expand All @@ -188,17 +197,22 @@ export async function selectPage (appIdKey, pageIdKey, skipReadyCheck = false) {
* @returns {Promise<import('../types').AppPage>}
*/
async function searchForApp (currentUrl, maxTries, ignoreAboutBlankUrl) {
const bundleIds = this._includeSafari && !this._isSafari
? [this._bundleId, ...this._additionalBundleIds, SAFARI_BUNDLE_ID]
: [this._bundleId, ...this._additionalBundleIds];
/** @type {string[]} */
const bundleIds = _.compact(
[
getBundleId(this),
...(getAdditionalBundleIds(this) ?? []),
...(getIncludeSafari(this) && !getIsSafari(this) ? [SAFARI_BUNDLE_ID] : []),
]
);
let retryCount = 0;
return /** @type {import('../types').AppPage} */ (await retryInterval(maxTries, SELECT_APP_RETRY_SLEEP_MS, async () => {
logApplicationDictionary.bind(this)();
const possibleAppIds = getPossibleDebuggerAppKeys.bind(this)(/** @type {string[]} */ (bundleIds));
this.log.debug(`Trying out the possible app ids: ${possibleAppIds.join(', ')} (try #${retryCount + 1} of ${maxTries})`);
for (const attemptedAppIdKey of possibleAppIds) {
try {
if (!this._appDict[attemptedAppIdKey].isActive) {
if (!getAppDict(this)[attemptedAppIdKey].isActive) {
this.log.debug(`Skipping app '${attemptedAppIdKey}' because it is not active`);
continue;
}
Expand All @@ -216,12 +230,12 @@ async function searchForApp (currentUrl, maxTries, ignoreAboutBlankUrl) {
}

// save the page array for this app
this._appDict[appIdKey].pageArray = pageArrayFromDict(pageDict);
getAppDict(this)[appIdKey].pageArray = pageArrayFromDict(pageDict);

// if we are looking for a particular url, make sure we
// have the right page. Ignore empty or undefined urls.
// Ignore about:blank if requested.
const result = searchForPage.bind(this)(this._appDict, currentUrl, ignoreAboutBlankUrl);
const result = searchForPage.bind(this)(getAppDict(this), currentUrl, ignoreAboutBlankUrl);
if (result) {
return result;
}
Expand Down Expand Up @@ -276,7 +290,7 @@ function searchForPage (appsDict, currentUrl = null, ignoreAboutBlankUrl = false
*/
function logApplicationDictionary () {
this.log.debug('Current applications available:');
for (const [app, info] of _.toPairs(this._appDict)) {
for (const [app, info] of _.toPairs(getAppDict(this))) {
this.log.debug(` Application: "${app}"`);
for (const [key, value] of _.toPairs(info)) {
if (key === 'pageArray' && Array.isArray(value) && value.length) {
Expand Down Expand Up @@ -307,7 +321,7 @@ function logApplicationDictionary () {
export function getPossibleDebuggerAppKeys(bundleIds) {
if (bundleIds.includes(WILDCARD_BUNDLE_ID)) {
this.log.debug('Skip checking bundle identifiers because the bundleIds includes a wildcard');
return _.uniq(Object.keys(this._appDict));
return _.uniq(Object.keys(getAppDict(this)));
}

// go through the possible bundle identifiers
Expand All @@ -324,10 +338,10 @@ export function getPossibleDebuggerAppKeys(bundleIds) {
const proxiedAppIds = new Set();
for (const bundleId of possibleBundleIds) {
// now we need to determine if we should pick a proxy for this instead
for (const appId of appIdsForBundle(bundleId, this._appDict)) {
for (const appId of appIdsForBundle(bundleId, getAppDict(this))) {
proxiedAppIds.add(appId);
this.log.debug(`Found app id key '${appId}' for bundle '${bundleId}'`);
for (const [key, data] of _.toPairs(this._appDict)) {
for (const [key, data] of _.toPairs(getAppDict(this))) {
if (data.isProxy && data.hostId === appId) {
this.log.debug(
`Found separate bundleId '${data.bundleId}' ` +
Expand All @@ -343,19 +357,5 @@ export function getPossibleDebuggerAppKeys(bundleIds) {
}

/**
* @typedef {Object} HasConnectionRelatedProperties
* @property {string | null | undefined} _appIdKey
* @property {string | number | null | undefined} _pageIdKey
* @property {import('../types').AppDict} _appDict
* @property {string | undefined} _bundleId
* @property {import('../rpc/rpc-client').RpcClient | undefined} _rpcClient
* @property {boolean} _includeSafari
* @property {boolean} _isSafari
* @property {string[]} _additionalBundleIds
* @property {(this: RemoteDebugger, timeoutMs?: number | undefined) => Promise<boolean>} checkPageIsReady:
* @property {(this: RemoteDebugger, startPageLoadTimer?: timing.Timer | null | undefined) => Promise<void>} waitForDom:
*/

/**
* @typedef {import('../remote-debugger').RemoteDebugger & HasConnectionRelatedProperties} RemoteDebugger
* @typedef {import('../remote-debugger').RemoteDebugger} RemoteDebugger
*/
26 changes: 12 additions & 14 deletions lib/mixins/cookies.js
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import {
getAppIdKey,
getPageIdKey,
} from './property-accessors';

/**
*
Expand All @@ -7,8 +11,8 @@
export async function getCookies () {
this.log.debug('Getting cookies');
return await this.requireRpcClient().send('Page.getCookies', {
appIdKey: this._appIdKey,
pageIdKey: this._pageIdKey
appIdKey: getAppIdKey(this),
pageIdKey: getPageIdKey(this),
});
}

Expand All @@ -21,9 +25,9 @@ export async function getCookies () {
export async function setCookie (cookie) {
this.log.debug('Setting cookie');
return await this.requireRpcClient().send('Page.setCookie', {
appIdKey: this._appIdKey,
pageIdKey: this._pageIdKey,
cookie
appIdKey: getAppIdKey(this),
pageIdKey: getPageIdKey(this),
cookie,
});
}

Expand All @@ -37,19 +41,13 @@ export async function setCookie (cookie) {
export async function deleteCookie (cookieName, url) {
this.log.debug(`Deleting cookie '${cookieName}' on '${url}'`);
return await this.requireRpcClient().send('Page.deleteCookie', {
appIdKey: this._appIdKey,
pageIdKey: this._pageIdKey,
appIdKey: getAppIdKey(this),
pageIdKey: getPageIdKey(this),
cookieName,
url,
});
}

/**
* @typedef {Object} HasCookiesRelatedProperties
* @property {string | null | undefined} _appIdKey
* @property {string | number | null | undefined} _pageIdKey
*/

/**
* @typedef {import('../remote-debugger').RemoteDebugger & HasCookiesRelatedProperties} RemoteDebugger
* @typedef {import('../remote-debugger').RemoteDebugger} RemoteDebugger
*/
21 changes: 10 additions & 11 deletions lib/mixins/events.js
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import {
getClientEventListeners,
} from './property-accessors';

// event emitted publically
export const events = {
export const events = /** @type {const} */ ({
EVENT_PAGE_CHANGE: 'remote_debugger_page_change',
EVENT_FRAMES_DETACHED: 'remote_debugger_frames_detached',
EVENT_DISCONNECT: 'remote_debugger_disconnect',
};
});

/**
* Keep track of the client event listeners so they can be removed
Expand All @@ -14,8 +18,8 @@ export const events = {
* @returns {void}
*/
export function addClientEventListener (eventName, listener) {
this._clientEventListeners[eventName] ??= [];
this._clientEventListeners[eventName].push(listener);
getClientEventListeners(this)[eventName] ??= [];
getClientEventListeners(this)[eventName].push(listener);
this.requireRpcClient().on(eventName, listener);
}

Expand All @@ -25,7 +29,7 @@ export function addClientEventListener (eventName, listener) {
* @returns {void}
*/
export function removeClientEventListener (eventName) {
for (const listener of (this._clientEventListeners[eventName] || [])) {
for (const listener of (getClientEventListeners(this)[eventName] || [])) {
this.requireRpcClient().off(eventName, listener);
}
}
Expand Down Expand Up @@ -73,10 +77,5 @@ export function stopNetwork () {
export default events;

/**
* @typedef {Object} HasEventsRelatedProperties
* @property {import('@appium/types').StringRecord<import('../types').EventListener[]>} _clientEventListeners:
*/

/**
* @typedef {import('../remote-debugger').RemoteDebugger & HasEventsRelatedProperties} RemoteDebugger
* @typedef {import('../remote-debugger').RemoteDebugger} RemoteDebugger
*/
Loading

0 comments on commit f47f6af

Please sign in to comment.