airbnb / airbnb/DeepLinkDispatch

Merge parameters parsing and match in a single method

Open
#66 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Kotlin
Stars
4.4k
Forks
414
PR merge metrics
No merged PRs in 30d

Description

DeepLinkDispatch is not very memory efficient when it comes to matching the uris. Say that you have 20 entries, building the registry will parse 20 pattern in the constructor even if all the entries are not used. As a workaround, `getParameters()` and `matches()` could be merged and return a new object containing all the parsed uri details. That way there's no need to create a regex and set of parameters when building the `DeepLinkEntry`. In terms of code, that's how it would look like:

DeepLinkRegistry:

``` java
public DeepLinkMatch parseUri(String uri) {
for (DeepLinkEntry entry : registry) {
final Optional match = entry.matches(uri);
if (match.isPresent()) {
return match.get();
}
}

return null;
}
```

DeepLinkEntry:

``` java
Optional matches(String inputUri) {
DeepLinkUri parsedUri = DeepLinkUri.parse(uri);
String schemeHostAndPath = schemeHostAndPath(parsedUri);
Set parameters = parsePathParameters(parsedUri);
Pattern regex = Pattern.compile(schemeHostAndPath.replaceAll(PARAM_REGEX, PARAM_VALUE) + "$");

DeepLinkUri inputParsedUri = DeepLinkUri.parse(inputUri);
boolean isMatch = inputParsedUri != null && regex.matcher(schemeHostAndPath(inputParsedUri)).find();

if (isMatch) {
Iterator paramsIterator = parameters.iterator();
Map paramsMap = new ArrayMap<>(parameters.size());
Matcher matcher = regex.matcher(schemeHostAndPath(inputParsedUri));
int i = 1;
if (matcher.matches()) {
while (paramsIterator.hasNext()) {
String key = paramsIterator.next();
String value = matcher.group(i++);
if (value != null && !"".equals(value.trim())) {
paramsMap.put(key, value);
}
}
}
return Optional.of(new DeepLinkMatch(this, paramsMap));

} else {
return Optional.absent();
}
}
```

DeepLinkMatch:

``` java
public class DeepLinkMatch {
private final DeepLinkEntry entry;
private final Map parameters;

public DeepLinkMatch(DeepLinkEntry entry, Map parameters) {
this.entry = entry;
this.parameters = parameters;
}

public DeepLinkEntry getEntry() {
return entry;
}

public Map getParameters() {
return parameters;
}
}
```

Do you guys agree with that and should I work on a PR? It would require a few changes in the processor.

Contributor guide

Open the contributing guide

Research direction

Start by reading the mentioned DeepLinkRegistry, DeepLinkEntry, and DeepLinkMatch classes, then trace how the processor uses them. The issue proposes merging URI matching and parameter parsing to avoid eagerly parsing every entry, but asks whether the approach is agreed on; confirm the intended design before implementation. No tests or file paths are named, so identify the relevant tests and use them to verify matching and parameter behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
android, kotlin
Domain
mobile-dev
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.