Skip to content
Open
Show file tree
Hide file tree
Changes from 13 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.UpdateService;
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 UpdateService 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;
}
231 changes: 231 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,231 @@
package org.cryptomator.linux.update;

import org.cryptomator.integrations.common.CheckAvailability;
import org.cryptomator.integrations.common.DistributionChannel;
import org.cryptomator.integrations.common.OperatingSystem;
import org.cryptomator.integrations.common.Priority;
import org.cryptomator.integrations.update.Progress;
import org.cryptomator.integrations.update.ProgressListener;
import org.cryptomator.integrations.update.SpawnExitedListener;
import org.cryptomator.integrations.update.SpawnStartedListener;
import org.cryptomator.integrations.update.UpdateAvailable;
import org.cryptomator.integrations.update.UpdateAvailableListener;
import org.cryptomator.integrations.update.UpdateFailedException;
import org.cryptomator.integrations.update.UpdateService;
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.purejava.portal.rest.UpdateCheckerTask;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CopyOnWriteArrayList;

@Priority(1000)
@CheckAvailability
@DistributionChannel(DistributionChannel.Value.LINUX_FLATPAK)
@OperatingSystem(OperatingSystem.Value.LINUX)
public class FlatpakUpdater implements UpdateService, AutoCloseable {

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

private final List<UpdateAvailableListener> updateAvailableListeners = new CopyOnWriteArrayList<>();
private final List<ProgressListener> progressListeners = new CopyOnWriteArrayList<>();
private final List<SpawnStartedListener> spawnStartedListeners = new CopyOnWriteArrayList<>();
private final List<SpawnExitedListener> spawnExitedListeners = new CopyOnWriteArrayList<>();

private final UpdatePortal portal;
private Flatpak.UpdateMonitor updateMonitor;

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.


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

@Override
public UpdateCheckerTask getLatestReleaseChecker(DistributionChannel.Value channel) {
if (channel != DistributionChannel.Value.LINUX_FLATPAK) {
LOG.error("Wrong channel provided: {}", channel);
return null;
}
portal.setUpdateCheckerTaskFor(APP_NAME);
return portal.getUpdateCheckerTaskFor(APP_NAME);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Throw exception instead of returning null for invalid channel.

Returning null for an invalid channel could lead to NullPointerException in the calling code. Consider throwing an IllegalArgumentException instead.

 @Override
 public UpdateCheckerTask getLatestReleaseChecker(DistributionChannel.Value channel) {
     if (channel != DistributionChannel.Value.LINUX_FLATPAK) {
-        LOG.error("Wrong channel provided: {}", channel);
-        return null;
+        throw new IllegalArgumentException("Wrong channel provided: " + channel + ". Expected: LINUX_FLATPAK");
     }
     portal.setUpdateCheckerTaskFor(APP_NAME);
     return portal.getUpdateCheckerTaskFor(APP_NAME);
 }
📝 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 UpdateCheckerTask getLatestReleaseChecker(DistributionChannel.Value channel) {
if (channel != DistributionChannel.Value.LINUX_FLATPAK) {
LOG.error("Wrong channel provided: {}", channel);
return null;
}
portal.setUpdateCheckerTaskFor(APP_NAME);
return portal.getUpdateCheckerTaskFor(APP_NAME);
}
@Override
public UpdateCheckerTask getLatestReleaseChecker(DistributionChannel.Value channel) {
if (channel != DistributionChannel.Value.LINUX_FLATPAK) {
throw new IllegalArgumentException("Wrong channel provided: " + channel + ". Expected: LINUX_FLATPAK");
}
portal.setUpdateCheckerTaskFor(APP_NAME);
return portal.getUpdateCheckerTaskFor(APP_NAME);
}
🤖 Prompt for AI Agents
In src/main/java/org/cryptomator/linux/update/FlatpakUpdater.java around lines
60 to 67, the method getLatestReleaseChecker returns null when an invalid
channel is provided, which risks causing NullPointerExceptions. Replace the null
return with throwing an IllegalArgumentException that clearly states the channel
is invalid. This change ensures callers are immediately informed of the misuse
and can handle the error properly.


@Override
public void triggerUpdate() throws UpdateFailedException {
var monitor = getUpdateMonitor();
portal.updateApp("x11:0", monitor, UpdatePortal.OPTIONS_DUMMY);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid hardcoding the display parameter.

The hardcoded "x11:0" display parameter may not work correctly in Wayland environments or other display configurations.

Consider obtaining the display parameter dynamically:

 @Override
 public void triggerUpdate() throws UpdateFailedException {
     var monitor = getUpdateMonitor();
-    portal.updateApp("x11:0", monitor, UpdatePortal.OPTIONS_DUMMY);
+    String display = System.getenv("DISPLAY");
+    if (display == null || display.isEmpty()) {
+        display = System.getenv("WAYLAND_DISPLAY");
+        if (display != null && !display.isEmpty()) {
+            display = "wayland:" + display;
+        } else {
+            display = "x11:0"; // fallback
+        }
+    }
+    portal.updateApp(display, monitor, 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 void triggerUpdate() throws UpdateFailedException {
var monitor = getUpdateMonitor();
portal.updateApp("x11:0", monitor, UpdatePortal.OPTIONS_DUMMY);
}
@Override
public void triggerUpdate() throws UpdateFailedException {
var monitor = getUpdateMonitor();
String display = System.getenv("DISPLAY");
if (display == null || display.isEmpty()) {
display = System.getenv("WAYLAND_DISPLAY");
if (display != null && !display.isEmpty()) {
display = "wayland:" + display;
} else {
display = "x11:0"; // fallback
}
}
portal.updateApp(display, monitor, UpdatePortal.OPTIONS_DUMMY);
}
🤖 Prompt for AI Agents
In src/main/java/org/cryptomator/linux/update/FlatpakUpdater.java around lines
70 to 73, the display parameter "x11:0" is hardcoded in the portal.updateApp
call, which can cause issues in Wayland or other display environments. Modify
the code to dynamically obtain the display parameter from the environment or
system properties instead of using a fixed string. Replace the hardcoded "x11:0"
with a variable that retrieves the current display setting at runtime.


@Override
public long spawnApp() {
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;

return spawnApp(cwdPath, argv, fds, envs, flags, options).longValue();
}

@Override
public boolean doesRequireElevatedPermissions() {
return false;
}

@Override
public String getDisplayName() {
return "Update via Flatpak update";
}

@Override
public void close() throws Exception {
try {
if (null != updateMonitor) {
portal.cancelUpdateMonitor(updateMonitor);
}
portal.close();
} catch (Exception e) {
LOG.error(e.toString(), e.getCause());
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve exception handling in close method.

The current implementation catches a generic Exception and the logging could be more informative.

 @Override
 public void close() throws Exception {
     try {
         if (null != updateMonitor) {
             portal.cancelUpdateMonitor(updateMonitor);
         }
         portal.close();
-    } catch (Exception e) {
-        LOG.error(e.toString(), e.getCause());
+    } catch (DBusException e) {
+        LOG.error("Failed to close DBus connection properly", e);
+        throw e;
+    } catch (Exception e) {
+        LOG.error("Unexpected error during resource cleanup", e);
+        throw e;
     }
 }
📝 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
@Override
public void close() throws Exception {
try {
if (null != updateMonitor) {
portal.cancelUpdateMonitor(updateMonitor);
}
portal.close();
} catch (Exception e) {
LOG.error(e.toString(), e.getCause());
}
}
@Override
public void close() throws Exception {
try {
if (null != updateMonitor) {
portal.cancelUpdateMonitor(updateMonitor);
}
portal.close();
} catch (DBusException e) {
LOG.error("Failed to close DBus connection properly", e);
throw e;
} catch (Exception e) {
LOG.error("Unexpected error during resource cleanup", e);
throw e;
}
}
🤖 Prompt for AI Agents
In src/main/java/org/cryptomator/linux/update/FlatpakUpdater.java around lines
98 to 108, improve the exception handling in the close method by catching more
specific exceptions if possible, and enhance the logging to provide a clearer
and more informative message. Instead of logging e.toString() and e.getCause()
separately, log the full exception with its stack trace to capture complete
context for debugging.


private synchronized Flatpak.UpdateMonitor getUpdateMonitor() {
if (updateMonitor == null) {
var updateMonitorPath = portal.CreateUpdateMonitor(UpdatePortal.OPTIONS_DUMMY);
if (updateMonitorPath != null) {
LOG.debug("UpdateMonitor successful created at {}", updateMonitorPath);
updateMonitor = portal.getUpdateMonitor(updateMonitorPath.toString());
try {
portal.getDBusConnection().addSigHandler(Flatpak.UpdateMonitor.UpdateAvailable.class, signal -> {
notifyOnUpdateAvailable(signal);
});
portal.getDBusConnection().addSigHandler(Flatpak.UpdateMonitor.Progress.class, signal -> {
notifyOnUpdateProceeds(signal);
});
} catch (DBusException e) {
LOG.error(e.toString(), e.getCause());
}
} else {
LOG.error("Failed to create UpdateMonitor on DBus");
}
}
return updateMonitor;
}

@Override
public void addUpdateAvailableListener(UpdateAvailableListener listener) {
updateAvailableListeners.add(listener);
}

@Override
public void removeUpdateAvailableListener(UpdateAvailableListener listener) {
updateAvailableListeners.remove(listener);
}

@Override
public void addProgressListener(ProgressListener listener) {
progressListeners.add(listener);
}

@Override
public void removeProgressListener(ProgressListener listener) {
progressListeners.remove(listener);
}

@Override
public void addSpawnStartedListener(SpawnStartedListener listener) {
spawnStartedListeners.add(listener);
}

@Override
public void removeSpawnStartedListener(SpawnStartedListener listener) {
spawnStartedListeners.remove(listener);
}

@Override
public void addSpawnExitedListener(SpawnExitedListener listener) {
spawnExitedListeners.add(listener);
}

@Override
public void removeSpawnExitedListener(SpawnExitedListener listener) {
spawnExitedListeners.remove(listener);
}

private void notifyOnUpdateAvailable(Flatpak.UpdateMonitor.UpdateAvailable signal) {
String remoteCommit = "";
Variant<?> remoteCommitVariant = signal.update_info.get("remote-commit");
if (null != remoteCommitVariant) {
remoteCommit = (String) remoteCommitVariant.getValue();
}
String runningCommit = "";
Variant<?> runningCommitVariant = signal.update_info.get("running-commit");
if (null != runningCommitVariant) {
runningCommit = (String) runningCommitVariant.getValue();
}
String localCommit = "";
Variant<?> localCommitVariant = signal.update_info.get("local-commit");
if (null != localCommitVariant) {
localCommit = (String) localCommitVariant.getValue();
}
UpdateAvailable updateAvailable = new UpdateAvailable(runningCommit, localCommit, remoteCommit);
for (UpdateAvailableListener listener : updateAvailableListeners) {
listener.onUpdateAvailable(updateAvailable);
}
}

private void notifyOnUpdateProceeds(Flatpak.UpdateMonitor.Progress signal) {
long status = ((UInt32) signal.info.get("status").getValue()).longValue();
long progress = 0;
Variant<?> progressVariant = signal.info.get("progress");
if (null != progressVariant) {
progress = ((UInt32) progressVariant.getValue()).longValue();
}
long nOps = -1;
Variant<?> nOpsVariant = signal.info.get("n_ops");
if (null != nOpsVariant) {
nOps = ((UInt32) nOpsVariant.getValue()).longValue();
}
long oP = -1;
Variant<?> oPVariant = signal.info.get("op");
if (null != oPVariant) {
oP = ((UInt32) oPVariant.getValue()).longValue();
}
String error = "";
Variant<?> errorVariant = signal.info.get("error");
if (null != errorVariant) {
error = (String) errorVariant.getValue();
}
String errorMessage = "";
Variant<?> errorMessageVariant = signal.info.get("error_message");
if (null != errorMessageVariant) {
errorMessage = (String) errorMessageVariant.getValue();
}
Progress p = new Progress(nOps, oP, status, progress, error, errorMessage);
for (ProgressListener listener : progressListeners) {
listener.onProgress(p);
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add type safety checks and reduce code duplication.

The direct cast without type checking could throw ClassCastException. Also, the code has repetitive patterns.

+private long extractLongFromVariant(Map<String, Variant<?>> map, String key, long defaultValue) {
+    Variant<?> variant = map.get(key);
+    if (variant != null && variant.getValue() instanceof UInt32) {
+        return ((UInt32) variant.getValue()).longValue();
+    }
+    return defaultValue;
+}
+
 private void notifyOnUpdateProceeds(Flatpak.UpdateMonitor.Progress signal) {
-    long status = ((UInt32) signal.info.get("status").getValue()).longValue();
-    long progress = 0;
-    Variant<?> progressVariant = signal.info.get("progress");
-    if (null != progressVariant) {
-        progress = ((UInt32) progressVariant.getValue()).longValue();
-    }
-    long nOps = -1;
-    Variant<?> nOpsVariant = signal.info.get("n_ops");
-    if (null != nOpsVariant) {
-        nOps = ((UInt32) nOpsVariant.getValue()).longValue();
-    }
-    long oP = -1;
-    Variant<?> oPVariant = signal.info.get("op");
-    if (null != oPVariant) {
-        oP = ((UInt32) oPVariant.getValue()).longValue();
-    }
-    String error = "";
-    Variant<?> errorVariant = signal.info.get("error");
-    if (null != errorVariant) {
-        error = (String) errorVariant.getValue();
-    }
-    String errorMessage = "";
-    Variant<?> errorMessageVariant = signal.info.get("error_message");
-    if (null != errorMessageVariant) {
-        errorMessage = (String) errorMessageVariant.getValue();
-    }
+    long status = extractLongFromVariant(signal.info, "status", 0);
+    long progress = extractLongFromVariant(signal.info, "progress", 0);
+    long nOps = extractLongFromVariant(signal.info, "n_ops", -1);
+    long oP = extractLongFromVariant(signal.info, "op", -1);
+    String error = extractStringFromVariant(signal.info, "error");
+    String errorMessage = extractStringFromVariant(signal.info, "error_message");
     Progress p = new Progress(nOps, oP, status, progress, error, errorMessage);
     for (ProgressListener listener : progressListeners) {
         listener.onProgress(p);
     }
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/main/java/org/cryptomator/linux/update/FlatpakUpdater.java lines 195 to
226, the method notifyOnUpdateProceeds directly casts values from signal.info
without type checks, risking ClassCastException, and repeats similar code
patterns. Refactor by adding type checks before casting to ensure safety, and
extract the repeated pattern of retrieving and casting values from the map into
a helper method to reduce duplication and improve readability.


private UInt32 spawnApp(List<Byte> cwdPath, List<List<Byte>> argv, Map<UInt32, FileDescriptor> fds, Map<String, String> envs, UInt32 flags, Map<String, Variant<?>> options) {
return portal.Spawn(cwdPath, argv, fds, envs, flags, options);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
org.cryptomator.linux.update.FlatpakUpdater