From fd7abdb8a43422d1c288ab2a57e921b421f10830 Mon Sep 17 00:00:00 2001 From: Matt Hill Date: Thu, 15 Dec 2022 12:00:01 -0700 Subject: [PATCH] don't be so fragile when comparing marketplace URLs (#2040) * don't be so fragile when comparing marketplace URLs * handle more edges * minor * clean up a little --- .../projects/shared/src/util/misc.util.ts | 28 +++++++++++-------- .../store-icon/store-icon.component.ts | 5 ++-- .../notifications-toast.service.ts | 4 +-- .../marketplace-settings.page.ts | 28 +++++++++++-------- .../marketplace-show-controls.component.ts | 9 ++++-- .../ui/src/app/pages/updates/updates.page.ts | 5 ++-- .../src/app/services/marketplace.service.ts | 3 +- 7 files changed, 49 insertions(+), 33 deletions(-) diff --git a/frontend/projects/shared/src/util/misc.util.ts b/frontend/projects/shared/src/util/misc.util.ts index 3da9cf8d5..faf3f3184 100644 --- a/frontend/projects/shared/src/util/misc.util.ts +++ b/frontend/projects/shared/src/util/misc.util.ts @@ -10,14 +10,6 @@ export function pauseFor(ms: number): Promise { return new Promise(resolve => setTimeout(resolve, ms)) } -export function capitalizeFirstLetter(string: string): string { - return string.charAt(0).toUpperCase() + string.slice(1) -} - -export function exists(t: T | undefined): t is T { - return t !== undefined -} - export function debounce(delay: number = 300): MethodDecorator { return function ( target: any, @@ -37,13 +29,16 @@ export function debounce(delay: number = 300): MethodDecorator { } } -export function removeTrailingSlash(word: string): string { - return word.replace(/\/+$/, '') +export function sameUrl( + u1: string | null | undefined, + u2: string | null | undefined, +): boolean { + return toUrl(u1) === toUrl(u2) } -export function isValidHttpUrl(string: string): boolean { +export function isValidHttpUrl(url: string): boolean { try { - const _ = new URL(string) + const _ = new URL(url) return true } catch (_) { return false @@ -53,3 +48,12 @@ export function isValidHttpUrl(string: string): boolean { export function getUrlHostname(url: string): string { return new URL(url).hostname } + +export function toUrl(text: string | null | undefined): string { + try { + const url = new URL(text as string) + return url.toString() + } catch { + return '' + } +} diff --git a/frontend/projects/ui/src/app/components/store-icon/store-icon.component.ts b/frontend/projects/ui/src/app/components/store-icon/store-icon.component.ts index 98ceb3143..0848e28fc 100644 --- a/frontend/projects/ui/src/app/components/store-icon/store-icon.component.ts +++ b/frontend/projects/ui/src/app/components/store-icon/store-icon.component.ts @@ -6,6 +6,7 @@ import { PipeTransform, } from '@angular/core' import { ConfigService } from 'src/app/services/config.service' +import { sameUrl } from '@start9labs/shared' @Component({ selector: 'store-icon', @@ -26,9 +27,9 @@ export class GetIconPipe implements PipeTransform { transform(url: string): string | null { const { start9, community } = this.config.marketplace - if (url === start9) { + if (sameUrl(url, start9)) { return 'assets/img/icon.png' - } else if (url === community) { + } else if (sameUrl(url, community)) { return 'assets/img/community-store.png' } return null diff --git a/frontend/projects/ui/src/app/components/toast-container/notifications-toast/notifications-toast.service.ts b/frontend/projects/ui/src/app/components/toast-container/notifications-toast/notifications-toast.service.ts index c9a6f30c6..52d35ea3c 100644 --- a/frontend/projects/ui/src/app/components/toast-container/notifications-toast/notifications-toast.service.ts +++ b/frontend/projects/ui/src/app/components/toast-container/notifications-toast/notifications-toast.service.ts @@ -1,7 +1,6 @@ import { Injectable } from '@angular/core' import { endWith, Observable } from 'rxjs' -import { filter, map, pairwise } from 'rxjs/operators' -import { exists } from '@start9labs/shared' +import { map, pairwise } from 'rxjs/operators' import { PatchDB } from 'patch-db-client' import { DataModel } from 'src/app/services/patch-db/data-model' @@ -10,7 +9,6 @@ export class NotificationsToastService extends Observable { private readonly stream$ = this.patch .watch$('server-info', 'unread-notification-count') .pipe( - filter(exists), pairwise(), map(([prev, cur]) => cur > prev), endWith(false), diff --git a/frontend/projects/ui/src/app/modals/marketplace-settings/marketplace-settings.page.ts b/frontend/projects/ui/src/app/modals/marketplace-settings/marketplace-settings.page.ts index b00266dfc..a2da04a65 100644 --- a/frontend/projects/ui/src/app/modals/marketplace-settings/marketplace-settings.page.ts +++ b/frontend/projects/ui/src/app/modals/marketplace-settings/marketplace-settings.page.ts @@ -6,7 +6,12 @@ import { ModalController, } from '@ionic/angular' import { ActionSheetButton } from '@ionic/core' -import { ErrorToastService } from '@start9labs/shared' +import { + ErrorToastService, + isValidHttpUrl, + sameUrl, + toUrl, +} from '@start9labs/shared' import { AbstractMarketplaceService } from '@start9labs/marketplace' import { ApiService } from 'src/app/services/api/embassy-api.service' import { ValueSpecObject } from 'src/app/pkg-config/config-types' @@ -31,7 +36,7 @@ export class MarketplaceSettingsPage { map(([stores, selected]) => { const toSlice = stores.map(s => ({ ...s, - selected: s.url === selected.url, + selected: sameUrl(s.url, selected.url), })) // 0 and 1 are prod and community const standard = toSlice.slice(0, 1) @@ -69,13 +74,13 @@ export class MarketplaceSettingsPage { { text: 'Save for Later', handler: (value: { url: string }) => { - this.saveOnly(new URL(value.url)) + this.saveOnly(value.url) }, }, { text: 'Save and Connect', handler: (value: { url: string }) => { - this.saveAndConnect(new URL(value.url)) + this.saveAndConnect(value.url) }, isSubmit: true, }, @@ -160,10 +165,11 @@ export class MarketplaceSettingsPage { } } - private async saveOnly(url: URL): Promise { + private async saveOnly(rawUrl: string): Promise { const loader = await this.loadingCtrl.create() try { + const url = new URL(rawUrl).toString() await this.validateAndSave(url, loader) } catch (e: any) { this.errToast.present(e) @@ -172,12 +178,13 @@ export class MarketplaceSettingsPage { } } - private async saveAndConnect(url: URL): Promise { + private async saveAndConnect(rawUrl: string): Promise { const loader = await this.loadingCtrl.create() try { + const url = new URL(rawUrl).toString() await this.validateAndSave(url, loader) - await this.connect(url.toString(), loader) + await this.connect(url, loader) } catch (e: any) { this.errToast.present(e) } finally { @@ -186,15 +193,14 @@ export class MarketplaceSettingsPage { } private async validateAndSave( - urlObj: URL, + url: string, loader: HTMLIonLoadingElement, ): Promise { - const url = urlObj.toString() // Error on duplicates const hosts = await firstValueFrom( this.patch.watch$('ui', 'marketplace', 'known-hosts'), ) - const currentUrls = Object.keys(hosts) + const currentUrls = Object.keys(hosts).map(toUrl) if (currentUrls.includes(url)) throw new Error('marketplace already added') // Validate @@ -225,7 +231,7 @@ export class MarketplaceSettingsPage { ) const filtered: { [url: string]: UIStore } = Object.keys(hosts) - .filter(key => key !== url) + .filter(key => !sameUrl(key, url)) .reduce((prev, curr) => { const name = hosts[curr] return { diff --git a/frontend/projects/ui/src/app/pages/marketplace-routes/marketplace-show/marketplace-show-controls/marketplace-show-controls.component.ts b/frontend/projects/ui/src/app/pages/marketplace-routes/marketplace-show/marketplace-show-controls/marketplace-show-controls.component.ts index 42a260d1b..d9e2d99d8 100644 --- a/frontend/projects/ui/src/app/pages/marketplace-routes/marketplace-show/marketplace-show-controls/marketplace-show-controls.component.ts +++ b/frontend/projects/ui/src/app/pages/marketplace-routes/marketplace-show/marketplace-show-controls/marketplace-show-controls.component.ts @@ -9,7 +9,12 @@ import { AbstractMarketplaceService, MarketplacePkg, } from '@start9labs/marketplace' -import { Emver, ErrorToastService, isEmptyObject } from '@start9labs/shared' +import { + Emver, + ErrorToastService, + isEmptyObject, + sameUrl, +} from '@start9labs/shared' import { DataModel, PackageDataEntry, @@ -71,7 +76,7 @@ export class MarketplaceShowControlsComponent { } else { const originalUrl = this.localPkg.installed?.['marketplace-url'] - if (url !== originalUrl) { + if (!sameUrl(url, originalUrl)) { const proceed = await this.presentAlertDifferentMarketplace( url, originalUrl, diff --git a/frontend/projects/ui/src/app/pages/updates/updates.page.ts b/frontend/projects/ui/src/app/pages/updates/updates.page.ts index 0804d44b2..5dfae2c9a 100644 --- a/frontend/projects/ui/src/app/pages/updates/updates.page.ts +++ b/frontend/projects/ui/src/app/pages/updates/updates.page.ts @@ -14,7 +14,7 @@ import { MarketplacePkg, StoreIdentity, } from '@start9labs/marketplace' -import { Emver, isEmptyObject } from '@start9labs/shared' +import { Emver, isEmptyObject, sameUrl } from '@start9labs/shared' import { Pipe, PipeTransform } from '@angular/core' import { combineLatest, Observable } from 'rxjs' import { PrimaryRendering } from '../../services/pkg-status-rendering.service' @@ -194,7 +194,8 @@ export function marketplaceSame( local: Record, url: string, ): boolean { - return local[id]?.installed?.['marketplace-url'] === url + const localUrl = local[id]?.installed?.['marketplace-url'] + return sameUrl(localUrl, url) } export function versionLower( diff --git a/frontend/projects/ui/src/app/services/marketplace.service.ts b/frontend/projects/ui/src/app/services/marketplace.service.ts index 8c74e9974..46dbce7e7 100644 --- a/frontend/projects/ui/src/app/services/marketplace.service.ts +++ b/frontend/projects/ui/src/app/services/marketplace.service.ts @@ -33,6 +33,7 @@ import { tap, } from 'rxjs/operators' import { ConfigService } from './config.service' +import { sameUrl } from '@start9labs/shared' @Injectable() export class MarketplaceService implements AbstractMarketplaceService { @@ -68,7 +69,7 @@ export class MarketplaceService implements AbstractMarketplaceService { startWith([]), pairwise(), mergeMap(([prev, curr]) => - curr.filter(c => !prev.find(p => c.url === p.url)), + curr.filter(c => !prev.find(p => sameUrl(c.url, p.url))), ), mergeMap(({ url, name }) => this.fetchStore$(url).pipe(