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.

Sign up for GitHub

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

Manage when Loading versions... is displayed and alert the user to when no versions are found #80

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Open

Manage when Loading versions... is displayed and alert the user to when no versions are found #80

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
66e9dd5
manage state
Nov 14, 2024
61e9ac5
Add generated changelog entries
svc-changelog Nov 14, 2024
b47cf85
schedule not tigger
Nov 14, 2024
c744f9a
Merge remote-tracking branch 'origin/develop' into finlayw/remove_loa…
Nov 14, 2024
2358cd7
merge
Nov 14, 2024
9f6c455
use aggeragte class
Nov 15, 2024
21fdd34
much tidyer
Nov 15, 2024
759e28f
hasSomeVersions
Nov 15, 2024
614952c
re-order function
Nov 15, 2024
ca5921f
debug log
Nov 15, 2024
a2ddc27
Merge branch 'develop' into finlayw/remove_loading
FinlayRJW Nov 15, 2024
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
11 changes: 1 addition & 10 deletions ...dea-plugin/src/main/java/com/palantir/gradle/versions/intellij/CompletionRefreshUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,18 @@

package com.palantir.gradle.versions.intellij;

import com.google.common.base.Suppliers;
import com.intellij.codeInsight.completion.BaseCompletionService;
import com.intellij.codeInsight.completion.CompletionProcess;
import com.intellij.codeInsight.completion.CompletionProgressIndicator;
import com.intellij.codeInsight.completion.CompletionService;
import com.intellij.openapi.application.ApplicationManager;
import java.util.function.Supplier;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class CompletionRefreshUtil {
private static final Logger log = LoggerFactory.getLogger(CompletionRefreshUtil.class);

public static Supplier<Void> refreshOnceSupplier() {
return Suppliers.memoize(() -> {
triggerRefresh();
return null;
});
}

private static void triggerRefresh() {
public static void triggerRefresh() {
ApplicationManager.getApplication().invokeLater(() -> {
CompletionService completionService = CompletionService.getCompletionService();
if (completionService == null) {
Expand Down
109 changes: 74 additions & 35 deletions ...gin/src/main/java/com/palantir/gradle/versions/intellij/VersionCompletionContributor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.palantir.gradle.versions.intellij;

import com.google.common.collect.EvictingQueue;
import com.intellij.codeInsight.completion.CompletionContributor;
import com.intellij.codeInsight.completion.CompletionParameters;
import com.intellij.codeInsight.completion.CompletionProvider;
Expand All @@ -31,13 +32,17 @@
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.ProcessingContext;
import com.palantir.gradle.versions.intellij.VersionExplorer.PackageInRepo;
import com.palantir.gradle.versions.intellij.VersionExplorer.VersionResults;
import com.palantir.gradle.versions.intellij.psi.VersionPropsDependencyVersion;
import com.palantir.gradle.versions.intellij.psi.VersionPropsProperty;
import com.palantir.gradle.versions.intellij.psi.VersionPropsTypes;
import java.util.AbstractMap.SimpleEntry;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.Objects;
import java.util.Queue;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.function.Function;
import java.util.stream.Collectors;
import one.util.streamex.StreamEx;
Expand All @@ -47,6 +52,9 @@ public class VersionCompletionContributor extends CompletionContributor {
GroupPartOrPackageNameExplorer.getInstance();
private final VersionExplorer versionExplorer = VersionExplorer.getInstance();

private static final int MAX_SIZE = 100;
private final Queue<DependencyInfo> loadedDependencies = EvictingQueue.create(MAX_SIZE);

VersionCompletionContributor() {
extend(
CompletionType.BASIC,
Expand All @@ -55,24 +63,25 @@ public class VersionCompletionContributor extends CompletionContributor {
@Override
public void addCompletions(
CompletionParameters parameters, ProcessingContext context, CompletionResultSet resultSet) {

DependencyInfo dependencyInfo = getDependencyInfo(parameters);
DependencyGroup group = dependencyInfo.group();
DependencyName dependencyName = dependencyInfo.dependencyName();

Project project = parameters.getOriginalFile().getProject();

CompletionSorter sorter = CompletionSorter.emptySorter().weigh(new VersionWeigher());
CompletionResultSet sortedResultSet = resultSet.withRelevanceSorter(sorter);

addLoadingElement(sortedResultSet);
if (!loadedDependencies.contains(dependencyInfo)) {
addDisplayElement(sortedResultSet, "Loading Versions...");
}

if (!dependencyName.name().contains("*")) {
handleDependencyWithoutWildcard(sortedResultSet, project, group, dependencyName);
if (!dependencyInfo.dependencyName().name().contains("*")) {
handleDependencyWithoutWildcard(sortedResultSet, project, dependencyInfo);
return;
}

List<PackageInRepo> groupAndDeps = collectGroupAndDeps(project, group, dependencyName);
addToResults(sortedResultSet, groupAndDeps);
List<PackageInRepo> packageInRepo = collectPackageInRepo(project, dependencyInfo);
addToResults(sortedResultSet, packageInRepo, dependencyInfo);
}
});
}
Expand All @@ -91,9 +100,9 @@ private DependencyInfo getDependencyInfo(CompletionParameters parameters) {
return new DependencyInfo(group, dependencyName);
}

private void addLoadingElement(CompletionResultSet sortedResultSet) {
private void addDisplayElement(CompletionResultSet sortedResultSet, String elementText) {
LookupElement loadingElement = PrioritizedLookupElement.withPriority(
LookupElementBuilder.create("Loading Versions...").withInsertHandler((elementContext, item) -> {
LookupElementBuilder.create(elementText).withInsertHandler((elementContext, item) -> {
// Prevent insertion
elementContext
.getDocument()
Expand All @@ -104,52 +113,82 @@ private void addLoadingElement(CompletionResultSet sortedResultSet) {
}

private void handleDependencyWithoutWildcard(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to handle these as a chunk so that we can be sure that there are no versions for all repos rather than just no versions for one repo

CompletionResultSet sortedResultSet,
Project project,
DependencyGroup group,
DependencyName dependencyName) {
RepositoryLoader.loadRepositories(project)
.forEach(url -> addToResults(sortedResultSet, List.of(new PackageInRepo(group, dependencyName, url))));
CompletionResultSet sortedResultSet, Project project, DependencyInfo dependencyInfo) {

List<PackageInRepo> allPackages = RepositoryLoader.loadRepositories(project).stream()
.map(url -> new PackageInRepo(dependencyInfo.group(), dependencyInfo.dependencyName(), url))
.collect(Collectors.toList());
addToResults(sortedResultSet, allPackages, dependencyInfo);
}

private List<PackageInRepo> collectGroupAndDeps(
Project project, DependencyGroup group, DependencyName dependencyName) {
private List<PackageInRepo> collectPackageInRepo(Project project, DependencyInfo dependencyInfo) {

String dependencyNamePrefix = dependencyName.name().replace("*", "");
String dependencyNamePrefix = dependencyInfo.dependencyName().name().replace("*", "");
return StreamEx.of(RepositoryLoader.loadRepositories(project))
.flatMap(url -> StreamEx.of(
groupPartOrPackageNameExplorer.getCancelableGroupPartOrPackageName(group, url))
.flatMap(url -> StreamEx.of(groupPartOrPackageNameExplorer.getCancelableGroupPartOrPackageName(
dependencyInfo.group(), url))
.filter(pkgName -> pkgName.name().startsWith(dependencyNamePrefix))
.map(pkgName -> new SimpleEntry<>(url, pkgName)))
.map(entry -> {
RepositoryUrl url = entry.getKey();
GroupPartOrPackageName pkgName = entry.getValue();
DependencyName depName = DependencyName.of(pkgName.name());
return new PackageInRepo(group, depName, url);
return new PackageInRepo(dependencyInfo.group(), depName, url);
})
.toList();
}

private void addToResults(CompletionResultSet resultSet, List<PackageInRepo> groupAndDeps) {
Map<DependencyVersion, AtomicInteger> versionCounts = StreamEx.of(groupAndDeps)
.flatMap(groupAndDep ->
versionExplorer
.getVersions(groupAndDep, CompletionRefreshUtil.refreshOnceSupplier()::get)
.stream())
.collect(Collectors.toConcurrentMap(
Function.identity(), v -> new AtomicInteger(1), (existingCount, newCount) -> {
existingCount.addAndGet(newCount.get());
return existingCount;
}));
private void addToResults(CompletionResultSet resultSet, List<PackageInRepo> packageInRepo, DependencyInfo key) {

Set<VersionResults> versionResults =
packageInRepo.stream().map(versionExplorer::getVersions).collect(Collectors.toSet());

List<CompletableFuture<?>> pendingFutures = versionResults.stream()
.filter(results -> results.versions().isEmpty())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually super strange to me (I guess I'm still thinking in the mindset where you give a List<PackageInRepo> to VersionExplorer#getVersions).

For each getVersions call, is it the case that it's either some already loaded set of versions or it's a future to get more? If so, we should actually structure VersionsResults as an choice between the two somehow. I think you can do this really nicely in Java 21, but in Java 17 I think you can do something like:

sealed interface VersionsResult permits AlreadyLoadedVersions, StillLoadingVersions {}

record AlreadyLoadedVersions(Set<DependencyVersion>)  {}
record StillLoadingVersions(CompletableFuture<Set<DependencyVersion>>)  {}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's definitely a type somewhere here that takes a set/list of VersionsResult and gives you methods like how many are still loading, how many versions are loaded etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I have made quite a few changes, I've switched getVersions to take a List<PackageInRepo> and now it returns a new type VersionsResults which handles the results aggregation. This makes addToResults alot neater as a lot of the logic is handled in the results type

.map(VersionResults::future)
.filter(future -> !future.isDone())
.collect(Collectors.toList());

if (!pendingFutures.isEmpty()) {
CompletableFuture.anyOf(pendingFutures.toArray(new CompletableFuture[0]))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should trigger a refresh when any one of the futures completes which should schedule less refreshes than just doing a refresh for each future I think

.thenAccept(completedFuture -> {
CompletionRefreshUtil.triggerRefresh();
});
}

Map<DependencyVersion, Integer> versionCounts = StreamEx.of(versionResults)
.flatMap(result -> result.versions().stream())
.collect(Collectors.toMap(Function.identity(), v -> 1, Integer::sum));

if (pendingFutures.isEmpty() && versionCounts.isEmpty()) {
addDisplayElement(resultSet, "No versions found");
addAndRefresh(key);
}

if (!versionCounts.isEmpty()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removes Loading Versions... from list of versions which is not strictly nessary as it sinks to the bottom but can be nicer when only a few versions are suggested

addAndRefresh(key);
}

Integer packageCount = packageInRepo.stream()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually want to count the number of unique artifact names and not the size of packageInRepo

.map(PackageInRepo::dependencyName)
.filter(Objects::nonNull)
.distinct()
.collect(Collectors.collectingAndThen(Collectors.counting(), Long::intValue));

List<LookupElement> lookupElements = versionCounts.entrySet().stream()
.map(entry ->
createLookupElement(entry.getKey(), entry.getValue().get(), groupAndDeps.size()))
.map(entry -> createLookupElement(entry.getKey(), entry.getValue(), packageCount))
.collect(Collectors.toList());

resultSet.addAllElements(lookupElements);
}

private void addAndRefresh(DependencyInfo key) {
if (!loadedDependencies.contains(key)) {
CompletionRefreshUtil.triggerRefresh();
loadedDependencies.add(key);
}
}

private LookupElement createLookupElement(DependencyVersion version, int count, int total) {
String typeText = ((total > 1) ? count + "/" + total + " packages" : "");
if (version.isLatest()) {
Expand Down
24 changes: 11 additions & 13 deletions ...ions-idea-plugin/src/main/java/com/palantir/gradle/versions/intellij/VersionExplorer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.Locale;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
Expand All @@ -54,23 +55,20 @@ public final class VersionExplorer {
.maximumSize(10000)
.buildAsync(this::fetchAndParseFromUrl);

public record PackageInRepo(DependencyGroup group, DependencyName dependencyPackage, RepositoryUrl repositoryUrl) {}
public record PackageInRepo(DependencyGroup group, DependencyName dependencyName, RepositoryUrl repositoryUrl) {}

public Set<DependencyVersion> getVersions(PackageInRepo groupAndDep, Runnable onLoadMore) {
String urlString = urlFor(groupAndDep);
public record VersionResults(Set<DependencyVersion> versions, CompletableFuture<Set<DependencyVersion>> future) {}

public VersionResults getVersions(PackageInRepo packageInRepo) {
String urlString = urlFor(packageInRepo);

Optional<Set<DependencyVersion>> cachedVersions =
Optional.ofNullable(shortLivedVersionCache.synchronous().getIfPresent(urlString));

if (cachedVersions.isPresent()) {
return cachedVersions.get();
}

shortLivedVersionCache.get(urlString).thenAccept(result -> {
onLoadMore.run();
});

return Collections.emptySet();
return cachedVersions
.map(dependencyVersions ->
new VersionResults(dependencyVersions, CompletableFuture.completedFuture(dependencyVersions)))
.orElseGet(() -> new VersionResults(Collections.emptySet(), shortLivedVersionCache.get(urlString)));
}

private Set<DependencyVersion> fetchAndParseFromUrl(String urlString) {
Expand All @@ -91,7 +89,7 @@ private Set<DependencyVersion> fetchAndParseFromUrl(String urlString) {

private static @NotNull String urlFor(PackageInRepo groupAndDep) {
return groupAndDep.repositoryUrl().url() + groupAndDep.group().asUrlString()
+ groupAndDep.dependencyPackage().name() + "/maven-metadata.xml";
+ groupAndDep.dependencyName().name() + "/maven-metadata.xml";
}

private Set<DependencyVersion> parseVersionsFromContent(String content) {
Expand Down