Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't be so fragile when comparing marketplace URLs #2040

Merged
merged 4 commits into from
Dec 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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