URI parsing improvements (#207)

* Prevent multiple parseURI calls

## Ticket
129

## Issue
Code was shortened to use `isURIValid` during the consolidation. `isURIValid` calls `normalizeURI`, which calls another `parseURI`.

`parseURI` is pretty expensive.

## Approach
- Add optional parameter to `isURIValid` to skip the normalization.
- Set those that were converted during the consolidation to skip the normalization. Also covered a few other instances where it is obvious to me that normalization is not required.
- For the rest, I can't tell for sure if it's safe to remove the normalization, so the default `normalize=true` will leave things as is.

The whole `parseURI` probably needs a refactoring, or a few lighter version for specific needs.

* Simplify isURIEqual

## Issue
`parseURI` is too expensive to be used in a loop, plus `normalizeURI` itself is calling `parseURI`.

## Approach
Not sure if it covers all cases, but just try convert colons to hashes before comparing.
This commit is contained in:
infinite-persistence 2021-11-03 00:37:53 +08:00 committed by GitHub
parent 704452732a
commit bf0aac2339
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 16 additions and 21 deletions

View file

@ -167,7 +167,7 @@ const ClaimPreview = forwardRef<any, {}>((props: Props, ref: any) => {
</span> </span>
); );
}, [channelSubCount]); }, [channelSubCount]);
const isValid = uri && isURIValid(uri); const isValid = uri && isURIValid(uri, false);
// $FlowFixMe // $FlowFixMe
const isPlayable = const isPlayable =

View file

@ -77,10 +77,11 @@ export const selectClaimForUri = createCachedSelector(
(state, uri) => uri, (state, uri) => uri,
(state, uri, returnRepost = true) => returnRepost, (state, uri, returnRepost = true) => returnRepost,
(byUri, byId, uri, returnRepost) => { (byUri, byId, uri, returnRepost) => {
const validUri = isURIValid(uri); const validUri = isURIValid(uri, false);
if (validUri && byUri) { if (validUri && byUri) {
const claimId = uri && byUri[normalizeURI(uri)]; const normalizedUri = normalizeURI(uri);
const claimId = uri && byUri[normalizedUri];
const claim = byId[claimId]; const claim = byId[claimId];
// Make sure to return the claim as is so apps can check if it's been resolved before (null) or still needs to be resolved (undefined) // Make sure to return the claim as is so apps can check if it's been resolved before (null) or still needs to be resolved (undefined)
@ -97,7 +98,7 @@ export const selectClaimForUri = createCachedSelector(
return { return {
...repostedClaim, ...repostedClaim,
repost_url: normalizeURI(uri), repost_url: normalizedUri,
repost_channel_url: channelUrl, repost_channel_url: channelUrl,
repost_bid_amount: claim && claim.meta && claim.meta.effective_amount, repost_bid_amount: claim && claim.meta && claim.meta.effective_amount,
}; };
@ -108,12 +109,14 @@ export const selectClaimForUri = createCachedSelector(
} }
)((state, uri, returnRepost = true) => `${String(uri)}:${returnRepost ? '1' : '0'}`); )((state, uri, returnRepost = true) => `${String(uri)}:${returnRepost ? '1' : '0'}`);
// Note: this is deprecated. Use "selectClaimForUri(state, uri)" instead.
export const makeSelectClaimForUri = (uri: string, returnRepost: boolean = true) => export const makeSelectClaimForUri = (uri: string, returnRepost: boolean = true) =>
createSelector(selectClaimIdsByUri, selectClaimsById, (byUri, byId) => { createSelector(selectClaimIdsByUri, selectClaimsById, (byUri, byId) => {
const validUri = isURIValid(uri); const validUri = isURIValid(uri, false);
if (validUri && byUri) { if (validUri && byUri) {
const claimId = uri && byUri[normalizeURI(uri)]; const normalizedUri = normalizeURI(uri);
const claimId = uri && byUri[normalizedUri];
const claim = byId[claimId]; const claim = byId[claimId];
// Make sure to return the claim as is so apps can check if it's been resolved before (null) or still needs to be resolved (undefined) // Make sure to return the claim as is so apps can check if it's been resolved before (null) or still needs to be resolved (undefined)
@ -130,7 +133,7 @@ export const makeSelectClaimForUri = (uri: string, returnRepost: boolean = true)
return { return {
...repostedClaim, ...repostedClaim,
repost_url: normalizeURI(uri), repost_url: normalizedUri,
repost_channel_url: channelUrl, repost_channel_url: channelUrl,
repost_bid_amount: claim && claim.meta && claim.meta.effective_amount, repost_bid_amount: claim && claim.meta && claim.meta.effective_amount,
}; };
@ -184,7 +187,7 @@ export const makeSelectClaimIsMine = (rawUri: string) => {
} catch (e) {} } catch (e) {}
return createSelector(selectClaimsByUri, selectMyActiveClaims, (claims, myClaims) => { return createSelector(selectClaimsByUri, selectMyActiveClaims, (claims, myClaims) => {
if (!isURIValid(uri)) { if (!isURIValid(uri, false)) {
return false; return false;
} }

View file

@ -265,9 +265,9 @@ export function normalizeURI(URL: string) {
}); });
} }
export function isURIValid(URL: string): boolean { export function isURIValid(URL: string, normalize: boolean = true): boolean {
try { try {
parseURI(normalizeURI(URL)); parseURI(normalize ? normalizeURI(URL) : URL);
} catch (error) { } catch (error) {
return false; return false;
} }
@ -323,15 +323,7 @@ export function splitBySeparator(uri: string) {
} }
export function isURIEqual(uriA: string, uriB: string) { export function isURIEqual(uriA: string, uriB: string) {
const parseA = parseURI(normalizeURI(uriA)); const a = uriA && uriA.replace(/:/g, '#');
const parseB = parseURI(normalizeURI(uriB)); const b = uriB && uriB.replace(/:/g, '#');
if (parseA.isChannel) { return a === b;
if (parseB.isChannel && parseA.channelClaimId === parseB.channelClaimId) {
return true;
}
} else if (parseA.streamClaimId === parseB.streamClaimId) {
return true;
} else {
return false;
}
} }