Skip to content

Feature/flatpak update portal #117

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

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 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
8 changes: 7 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@

<!-- runtime dependencies -->

<api.version>1.6.0</api.version>
<api.version>1.7.0-SNAPSHOT</api.version>
<secret-service.version>2.0.1-alpha</secret-service.version>
<kdewallet.version>1.4.0</kdewallet.version>
<flatpakupdateportal.version>1.0.0</flatpakupdateportal.version>
<slf4j.version>2.0.17</slf4j.version>
<appindicator.version>1.4.2</appindicator.version>

Expand Down Expand Up @@ -88,6 +89,11 @@
<artifactId>libappindicator-gtk3-java-minimal</artifactId>
<version>${appindicator.version}</version>
</dependency>
<dependency>
<groupId>org.purejava</groupId>
<artifactId>flatpak-update-portal</artifactId>
<version>${flatpakupdateportal.version}</version>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
Expand Down
7 changes: 6 additions & 1 deletion src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,23 @@
import org.cryptomator.integrations.quickaccess.QuickAccessService;
import org.cryptomator.integrations.revealpath.RevealPathService;
import org.cryptomator.integrations.tray.TrayMenuController;
import org.cryptomator.integrations.update.UpdateMechanism;
import org.cryptomator.linux.autostart.FreedesktopAutoStartService;
import org.cryptomator.linux.keychain.KDEWalletKeychainAccess;
import org.cryptomator.linux.keychain.GnomeKeyringKeychainAccess;
import org.cryptomator.linux.keychain.KDEWalletKeychainAccess;
import org.cryptomator.linux.quickaccess.DolphinPlaces;
import org.cryptomator.linux.quickaccess.NautilusBookmarks;
import org.cryptomator.linux.revealpath.DBusSendRevealPathService;
import org.cryptomator.linux.tray.AppindicatorTrayMenuController;
import org.cryptomator.linux.update.FlatpakUpdater;

module org.cryptomator.integrations.linux {
requires org.cryptomator.integrations.api;
requires org.slf4j;
requires org.freedesktop.dbus;
requires org.purejava.appindicator;
requires org.purejava.kwallet;
requires org.purejava.portal;
requires de.swiesend.secretservice;
requires java.xml;

Expand All @@ -25,8 +28,10 @@
provides RevealPathService with DBusSendRevealPathService;
provides TrayMenuController with AppindicatorTrayMenuController;
provides QuickAccessService with NautilusBookmarks, DolphinPlaces;
provides UpdateMechanism with FlatpakUpdater;

opens org.cryptomator.linux.tray to org.cryptomator.integrations.api;
opens org.cryptomator.linux.quickaccess to org.cryptomator.integrations.api;
opens org.cryptomator.linux.autostart to org.cryptomator.integrations.api;
opens org.cryptomator.linux.update to org.cryptomator.integrations.api;
}
202 changes: 202 additions & 0 deletions src/main/java/org/cryptomator/linux/update/FlatpakUpdater.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
package org.cryptomator.linux.update;

import org.cryptomator.integrations.common.CheckAvailability;
import org.cryptomator.integrations.common.DisplayName;
import org.cryptomator.integrations.common.OperatingSystem;
import org.cryptomator.integrations.common.Priority;
import org.cryptomator.integrations.update.UpdateFailedException;
import org.cryptomator.integrations.update.UpdateMechanism;
import org.cryptomator.integrations.update.UpdateProcess;
import org.freedesktop.dbus.FileDescriptor;
import org.freedesktop.dbus.exceptions.DBusException;
import org.freedesktop.dbus.types.UInt32;
import org.freedesktop.dbus.types.Variant;
import org.purejava.portal.Flatpak;
import org.purejava.portal.FlatpakSpawnFlag;
import org.purejava.portal.UpdatePortal;
import org.purejava.portal.Util;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;

@Priority(1000)
@CheckAvailability
@DisplayName("Update via Flatpak update")
@OperatingSystem(OperatingSystem.Value.LINUX)
public class FlatpakUpdater implements UpdateMechanism {

private static final Logger LOG = LoggerFactory.getLogger(FlatpakUpdater.class);
private static final String APP_NAME = "org.cryptomator.Cryptomator";

private final UpdatePortal portal;

public FlatpakUpdater() {
this.portal = new UpdatePortal();
portal.CreateUpdateMonitor(UpdatePortal.OPTIONS_DUMMY);
}
Comment on lines +41 to +44
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove unnecessary monitor creation in constructor.

The constructor creates an update monitor but doesn't check or use the result. This monitor should be created lazily when first needed via getUpdateMonitor().

 public FlatpakUpdater() {
 	this.portal = new UpdatePortal();
-	portal.CreateUpdateMonitor(UpdatePortal.OPTIONS_DUMMY);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public FlatpakUpdater() {
this.portal = new UpdatePortal();
portal.CreateUpdateMonitor(UpdatePortal.OPTIONS_DUMMY);
}
public FlatpakUpdater() {
this.portal = new UpdatePortal();
}
🤖 Prompt for AI Agents
In src/main/java/org/cryptomator/linux/update/FlatpakUpdater.java between lines
42 and 45, remove the call to
portal.CreateUpdateMonitor(UpdatePortal.OPTIONS_DUMMY) from the constructor
since the update monitor is created but never used there. Instead, implement
lazy initialization of the update monitor inside the getUpdateMonitor() method
so that the monitor is only created when first accessed.


@CheckAvailability
public boolean isSupported() {
return portal.isAvailable();
}

@Override
public boolean isUpdateAvailable() {
var cdl = new CountDownLatch(1);
portal.setUpdateCheckerTaskFor(APP_NAME);
var checkTask = portal.getUpdateCheckerTaskFor(APP_NAME);
var updateAvailable = new AtomicBoolean(false);
checkTask.setOnSucceeded(latestVersion -> {
updateAvailable.set(true); // TODO: compare version strings before setting this to true
cdl.countDown();
});
checkTask.setOnFailed(error -> {
LOG.warn("Error while checking for updates.", error);
cdl.countDown();
});
try {
cdl.await();
return updateAvailable.get();
} catch (InterruptedException e) {
checkTask.cancel();
Thread.currentThread().interrupt();
return false;
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add timeout and proper version comparison in update check

isUpdateAvailable() blocks indefinitely on cdl.await() and sets the flag to true without validating that latestVersion is newer than the running one.
• Specify a sensible timeout (e.g. 30 s) and abort gracefully.
• Compare latestVersion with BuildConfig.version() (or similar) before returning true.

🤖 Prompt for AI Agents
In src/main/java/org/cryptomator/linux/update/FlatpakUpdater.java between lines
51 and 73, the isUpdateAvailable() method currently blocks indefinitely on
cdl.await() and sets updateAvailable to true without verifying if the
latestVersion is actually newer than the current version. To fix this, add a
timeout (e.g., 30 seconds) to the cdl.await() call to prevent indefinite
blocking and handle the timeout case gracefully. Also, modify the onSucceeded
handler to compare the received latestVersion string with the current version
from BuildConfig.version() (or equivalent) and only set updateAvailable to true
if the latestVersion is newer.


@Override
public UpdateProcess prepareUpdate() throws UpdateFailedException {
var monitorPath = portal.CreateUpdateMonitor(UpdatePortal.OPTIONS_DUMMY);
if (monitorPath == null) {
throw new UpdateFailedException("Failed to create UpdateMonitor on DBus");
}

return new FlatpakUpdateProcess(portal.getUpdateMonitor(monitorPath.toString()));
}

private class FlatpakUpdateProcess implements UpdateProcess {

private final CountDownLatch latch = new CountDownLatch(1);
private final Flatpak.UpdateMonitor monitor;
private volatile double progress = 0.0;
private volatile UpdateFailedException error;
private AutoCloseable signalHandler;

private FlatpakUpdateProcess(Flatpak.UpdateMonitor monitor) {
this.monitor = monitor;
startUpdate();
}

private void startUpdate() {
try {
this.signalHandler = portal.getDBusConnection().addSigHandler(Flatpak.UpdateMonitor.Progress.class, this::handleProgressSignal);
} catch (DBusException e) {
LOG.error("DBus error", e);
latch.countDown();
}
portal.updateApp("x11:0", monitor, UpdatePortal.OPTIONS_DUMMY);
}

private void handleProgressSignal(Flatpak.UpdateMonitor.Progress signal) {
int status = ((UInt32) signal.info.get("status").getValue()).intValue();
switch (status) {
case 0 -> { // In progress
Variant<?> progressVariant = signal.info.get("progress");
if (progressVariant != null) {
progress = ((UInt32) progressVariant.getValue()).doubleValue() / 100.0; // progress reported as int in range [0, 100]
}
}
case 1 -> { // No update available
error = new UpdateFailedException("No update available");
latch.countDown();
}
case 2 -> { // Update complete
progress = 1.0;
latch.countDown();
}
case 3 -> { // Update failed
error = new UpdateFailedException("Update preparation failed");
latch.countDown();
}
default -> {
error = new UpdateFailedException("Unknown update status " + status);
latch.countDown();
}
}
}

private void stopReceivingSignals() {
if (signalHandler != null) {
try {
signalHandler.close();
} catch (Exception e) {
LOG.error("Failed to close signal handler", e);
}
signalHandler = null;
}
}

@Override
public double preparationProgress() {
return progress;
}

@Override
public void cancel() {
portal.cancelUpdateMonitor(monitor);
stopReceivingSignals();
portal.close(); // TODO: is this right? belongs to parent class. update can not be retried afterwards. or should each process have its own portal instance?
error = new UpdateFailedException("Update cancelled by user");
}
Comment on lines +154 to +158
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Portal lifecycle & resource leak

cancel() closes the shared portal, but the normal success path (applyUpdate) never does, leaving the DBus connection open for the lifetime of the JVM. Either:
• Make FlatpakUpdateProcess own a dedicated UpdatePortal and always close it in finally, or
• Let FlatpakUpdater implement AutoCloseable and close the portal when the application shuts down.

🤖 Prompt for AI Agents
In src/main/java/org/cryptomator/linux/update/FlatpakUpdater.java around lines
154 to 158, the shared portal is closed on cancel but not on successful update,
causing a resource leak. To fix this, either refactor FlatpakUpdateProcess to
own a dedicated UpdatePortal instance and ensure it is closed in a finally block
after update attempts, or modify FlatpakUpdater to implement AutoCloseable and
close the shared portal when the application shuts down, ensuring the DBus
connection is properly released in all cases.


@Override
public void await() throws InterruptedException {
latch.await();
}

@Override
public boolean await(long timeout, TimeUnit unit) throws InterruptedException {
return latch.await(timeout, unit);
}

private boolean isDone() {
try {
return latch.await(0, TimeUnit.MILLISECONDS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
return false;
}
}

@Override
public ProcessHandle applyUpdate() throws IllegalStateException, IOException {
if (!isDone()) {
throw new IllegalStateException("Update preparation is not complete");
}
stopReceivingSignals();
if (error != null) {
throw error;
}

// spawn new Cryptomator process:
var cwdPath = Util.stringToByteList(System.getProperty("user.dir"));
List<List<Byte>> argv = List.of(
Util.stringToByteList(APP_NAME));
Map<UInt32, FileDescriptor> fds = Collections.emptyMap();
Map<String, String> envs = Map.of();
UInt32 flags = new UInt32(FlatpakSpawnFlag.LATEST_VERSION.getValue());
Map<String, Variant<?>> options = UpdatePortal.OPTIONS_DUMMY;
var pid = portal.Spawn(cwdPath, argv, fds, envs, flags, options).longValue();
return ProcessHandle.of(pid).orElseThrow();
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
org.cryptomator.linux.update.FlatpakUpdater