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.

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

Feature: Autostart for DE following the freedesktop standard #80

Merged
merged 4 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ Linux-specific implemenations of the [integrations-api](https://github.com/crypt

This project uses the following JVM properties:
* `cryptomator.integrationsLinux.trayIconsDir` - specifies the directory from which svg images for the tray icon are loaded
infeo marked this conversation as resolved.
Show resolved Hide resolved

* `cryptomator.integrationsLinux.autoStartCmd` - specifies the command used for starting Cryptomator
Copy link

Choose a reason for hiding this comment

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

Ensure the file ends with a single newline character.

The file should end with a single newline character.

- * `cryptomator.integrationsLinux.autoStartCmd` - specifies the command used for starting Cryptomator
+ * `cryptomator.integrationsLinux.autoStartCmd` - specifies the command used for starting Cryptomator
+
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
* `cryptomator.integrationsLinux.autoStartCmd` - specifies the command used for starting Cryptomator
* `cryptomator.integrationsLinux.autoStartCmd` - specifies the command used for starting Cryptomator
Tools
Markdownlint

8-8: Expected: 0 or 2; Actual: 1
Trailing spaces

(MD009, no-trailing-spaces)


8-8: null
Files should end with a single newline character

(MD047, single-trailing-newline)


Remove trailing spaces.

There is a trailing space at the end of the line.

- * `cryptomator.integrationsLinux.autoStartCmd` - specifies the command used for starting Cryptomator 
+ * `cryptomator.integrationsLinux.autoStartCmd` - specifies the command used for starting Cryptomator
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
* `cryptomator.integrationsLinux.autoStartCmd` - specifies the command used for starting Cryptomator
* `cryptomator.integrationsLinux.autoStartCmd` - specifies the command used for starting Cryptomator
Tools
Markdownlint

8-8: Expected: 0 or 2; Actual: 1
Trailing spaces

(MD009, no-trailing-spaces)


8-8: null
Files should end with a single newline character

(MD047, single-trailing-newline)

infeo marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions src/main/java/module-info.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import org.cryptomator.integrations.autostart.AutoStartProvider;
import org.cryptomator.integrations.keychain.KeychainAccessProvider;
import org.cryptomator.integrations.revealpath.RevealPathService;
import org.cryptomator.integrations.tray.TrayMenuController;
import org.cryptomator.linux.autostart.FreedesktopAutoStartService;
import org.cryptomator.linux.keychain.KDEWalletKeychainAccess;
import org.cryptomator.linux.keychain.SecretServiceKeychainAccess;
import org.cryptomator.linux.revealpath.DBusSendRevealPathService;
Expand All @@ -14,6 +16,7 @@
requires org.purejava.kwallet;
requires de.swiesend.secretservice;

provides AutoStartProvider with FreedesktopAutoStartService;
provides KeychainAccessProvider with SecretServiceKeychainAccess, KDEWalletKeychainAccess;
provides RevealPathService with DBusSendRevealPathService;
provides TrayMenuController with AppindicatorTrayMenuController;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package org.cryptomator.linux.autostart;

import org.cryptomator.integrations.autostart.AutoStartProvider;
import org.cryptomator.integrations.autostart.ToggleAutoStartFailedException;
import org.cryptomator.integrations.common.CheckAvailability;
import org.cryptomator.integrations.common.OperatingSystem;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.util.Objects;

/**
* Enables autostart for Linux desktop environments following the freedesktop standard.
* <p>
* This service is based on <a href=https://specifications.freedesktop.org/autostart-spec/autostart-spec-0.5.html>version 0.5 of the freedesktop autostart-spec</a>.
*/
@CheckAvailability
@OperatingSystem(OperatingSystem.Value.LINUX)
public class FreedesktopAutoStartService implements AutoStartProvider {

private static final Logger LOG = LoggerFactory.getLogger(FreedesktopAutoStartService.class);
private static final String CMD_PROPERTY = "cryptomator.integrationsLinux.autoStartCmd";
private static final String AUTOSTART_FILENAME = "Cryptomator.desktop";
private static final String CONTENT_TEMPLATE = """
[Desktop Entry]
Type=Application
Exec=%s
Hidden=false
NoDisplay=false
X-GNOME-Autostart-enabled=true
Name=Cryptomator
Comment=Created with %s
""";

private final Path autostartFile;
private final String content;

public FreedesktopAutoStartService() {
var xdgConfigDirString = Objects.requireNonNullElse(System.getenv("XDG_CONFIG_HOME"), System.getProperty("user.home") + "/.config");
this.autostartFile = Path.of(xdgConfigDirString, "autostart", AUTOSTART_FILENAME);

var execValue = System.getProperty(CMD_PROPERTY);
if (execValue == null) {
LOG.debug("Property {} not set, using command path", CMD_PROPERTY);
execValue = ProcessHandle.current().info().command().orElse("");
}
this.content = CONTENT_TEMPLATE.formatted(execValue, this.getClass().getName());
}

@Override
public void enable() throws ToggleAutoStartFailedException {
try {
Files.writeString(autostartFile, content, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING);
} catch (IOException e) {
throw new ToggleAutoStartFailedException("Failed to activate Cryptomator autostart for GNOME desktop environment.", e);
}
}
Copy link

Choose a reason for hiding this comment

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

Add logging for exceptions in enable method.

While the method correctly handles IOException by throwing ToggleAutoStartFailedException, consider logging the exception details for better debugging.

-    throw new ToggleAutoStartFailedException("Failed to activate Cryptomator autostart for GNOME desktop environment.", e);
+    LOG.error("Failed to write autostart file", e);
+    throw new ToggleAutoStartFailedException("Failed to activate Cryptomator autostart for GNOME desktop environment.", 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 enable() throws ToggleAutoStartFailedException {
try {
Files.writeString(autostartFile, content, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING);
} catch (IOException e) {
throw new ToggleAutoStartFailedException("Failed to activate Cryptomator autostart for GNOME desktop environment.", e);
}
}
@Override
public void enable() throws ToggleAutoStartFailedException {
try {
Files.writeString(autostartFile, content, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING);
} catch (IOException e) {
LOG.error("Failed to write autostart file", e);
throw new ToggleAutoStartFailedException("Failed to activate Cryptomator autostart for GNOME desktop environment.", e);
}
}


@Override
public void disable() throws ToggleAutoStartFailedException {
try {
Files.deleteIfExists(autostartFile);
} catch (IOException e) {
throw new ToggleAutoStartFailedException("Failed to deactivate Cryptomator autostart for GNOME desktop environment.", e);
}
}
Copy link

Choose a reason for hiding this comment

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

Add logging for exceptions in disable method.

While the method correctly handles IOException by throwing ToggleAutoStartFailedException, consider logging the exception details for better debugging.

-    throw new ToggleAutoStartFailedException("Failed to deactivate Cryptomator autostart for GNOME desktop environment.", e);
+    LOG.error("Failed to delete autostart file", e);
+    throw new ToggleAutoStartFailedException("Failed to deactivate Cryptomator autostart for GNOME desktop environment.", 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 disable() throws ToggleAutoStartFailedException {
try {
Files.deleteIfExists(autostartFile);
} catch (IOException e) {
throw new ToggleAutoStartFailedException("Failed to deactivate Cryptomator autostart for GNOME desktop environment.", e);
}
}
@Override
public void disable() throws ToggleAutoStartFailedException {
try {
Files.deleteIfExists(autostartFile);
} catch (IOException e) {
LOG.error("Failed to delete autostart file", e);
throw new ToggleAutoStartFailedException("Failed to deactivate Cryptomator autostart for GNOME desktop environment.", e);
}
}


@Override
public boolean isEnabled() {
return Files.exists(autostartFile);
}

@CheckAvailability
public boolean isSupported() {
//TODO: might need to research which Desktop Environments support this
return Files.exists(autostartFile.getParent());
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
org.cryptomator.linux.autostart.FreedesktopAutoStartService
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package org.cryptomator.linux.autostart;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.MethodOrderer;
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestMethodOrder;

@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
public class FreeDesktopAutoStartIT {
Copy link

Choose a reason for hiding this comment

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

Class name should be consistent.

The class name FreeDesktopAutoStartIT should be consistent with the service class name FreedesktopAutoStartService.

- public class FreeDesktopAutoStartIT {
+ public class FreedesktopAutoStartIT {
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 class FreeDesktopAutoStartIT {
public class FreedesktopAutoStartIT {


FreedesktopAutoStartService inTest = new FreedesktopAutoStartService();

@Test
@Order(1)
public void testAutostartEnable() {
Assertions.assertDoesNotThrow(() -> inTest.enable());
Assertions.assertTrue(inTest.isEnabled());
}


@Test
@Order(2)
public void testAutostartDisable() {
Assertions.assertDoesNotThrow(() -> inTest.disable());
Assertions.assertFalse(inTest.isEnabled());
}
}