Skip to content

Commit

Permalink
don't be so fragile when comparing marketplace URLs (#2040)
Browse files Browse the repository at this point in the history
* don't be so fragile when comparing marketplace URLs

* handle more edges

* minor

* clean up a little
  • Loading branch information
MattDHill authored Dec 15, 2022
1 parent 92cd85b commit fd7abdb
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 33 deletions.
28 changes: 16 additions & 12 deletions frontend/projects/shared/src/util/misc.util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,6 @@ export function pauseFor(ms: number): Promise<void> {
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: T | undefined): t is T {
return t !== undefined
}

export function debounce(delay: number = 300): MethodDecorator {
return function (
target: any,
Expand All @@ -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
Expand All @@ -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 ''
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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'

Expand All @@ -10,7 +9,6 @@ export class NotificationsToastService extends Observable<boolean> {
private readonly stream$ = this.patch
.watch$('server-info', 'unread-notification-count')
.pipe(
filter(exists),
pairwise(),
map(([prev, cur]) => cur > prev),
endWith(false),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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)
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -160,10 +165,11 @@ export class MarketplaceSettingsPage {
}
}

private async saveOnly(url: URL): Promise<void> {
private async saveOnly(rawUrl: string): Promise<void> {
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)
Expand All @@ -172,12 +178,13 @@ export class MarketplaceSettingsPage {
}
}

private async saveAndConnect(url: URL): Promise<void> {
private async saveAndConnect(rawUrl: string): Promise<void> {
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 {
Expand All @@ -186,15 +193,14 @@ export class MarketplaceSettingsPage {
}

private async validateAndSave(
urlObj: URL,
url: string,
loader: HTMLIonLoadingElement,
): Promise<void> {
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
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions frontend/projects/ui/src/app/pages/updates/updates.page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -194,7 +194,8 @@ export function marketplaceSame(
local: Record<string, PackageDataEntry>,
url: string,
): boolean {
return local[id]?.installed?.['marketplace-url'] === url
const localUrl = local[id]?.installed?.['marketplace-url']
return sameUrl(localUrl, url)
}

export function versionLower(
Expand Down
3 changes: 2 additions & 1 deletion frontend/projects/ui/src/app/services/marketplace.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -68,7 +69,7 @@ export class MarketplaceService implements AbstractMarketplaceService {
startWith<StoreIdentity[]>([]),
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(
Expand Down

0 comments on commit fd7abdb

Please sign in to comment.