From 3a571c14f9be43a2fdf7c837c23f7df0e64a0113 Mon Sep 17 00:00:00 2001 From: Sergio Costas Date: Fri, 16 Feb 2024 08:44:56 +0100 Subject: [PATCH] agentnotify: show the snap icon when autorefresh is done (#13486) * agentnotify: show the snap icon when autorefresh is done When an application has to be closed by the user to allow to do and autorefresh, the notification has the application icon. But when the autorefresh has been completed, the notification has the snapcraft icon instead. This MR fixes this. With it, the notification specifying that the snap has been successfully refreshed and that it can be relaunched shows the application's icon. Fix https://warthogs.atlassian.net/browse/UDENG-2030 * Fix .desktop file name generation * Use an heuristic to determine the desktop file * Move all the logic to the user-space daemon * Don't fail if ReadCurrentInfo fails Also, really take the first icon found, and not the last one. * Moved code into function and added tests * Update usersession/agent/rest_api.go Co-authored-by: Maciej Borzecki * Update usersession/agent/rest_api.go Co-authored-by: Maciej Borzecki * Extra changes requested by reviewers * Add extra test * Extra changes requested * Update usersession/agent/rest_api_test.go Co-authored-by: Maciej Borzecki * Some style patches --------- Co-authored-by: Maciej Borzecki --- snap/info.go | 3 +- usersession/agent/export_test.go | 1 + usersession/agent/rest_api.go | 42 ++++++++ usersession/agent/rest_api_test.go | 148 +++++++++++++++++++++++++++++ 4 files changed, 192 insertions(+), 2 deletions(-) diff --git a/snap/info.go b/snap/info.go index 71ebd2eae9e..1ef739c7ed1 100644 --- a/snap/info.go +++ b/snap/info.go @@ -23,7 +23,6 @@ import ( "bytes" "encoding/json" "fmt" - "io/ioutil" "net/url" "os" "path/filepath" @@ -1397,7 +1396,7 @@ func ReadInfo(name string, si *SideInfo) (*Info, error) { // snap given the mound point, mount file, and side info. func ReadInfoFromMountPoint(name, mountPoint, mountFile string, si *SideInfo) (*Info, error) { snapYamlFn := filepath.Join(mountPoint, "meta", "snap.yaml") - meta, err := ioutil.ReadFile(snapYamlFn) + meta, err := os.ReadFile(snapYamlFn) if os.IsNotExist(err) { return nil, &NotFoundError{Snap: name, Revision: si.Revision, Path: snapYamlFn} } diff --git a/usersession/agent/export_test.go b/usersession/agent/export_test.go index 9514b506049..7e93e601fbf 100644 --- a/usersession/agent/export_test.go +++ b/usersession/agent/export_test.go @@ -29,6 +29,7 @@ var ( ServiceStatusCmd = serviceStatusCmd PendingRefreshNotificationCmd = pendingRefreshNotificationCmd FinishRefreshNotificationCmd = finishRefreshNotificationCmd + GuessAppIcon = guessAppIcon ) func MockUcred(ucred *syscall.Ucred, err error) (restore func()) { diff --git a/usersession/agent/rest_api.go b/usersession/agent/rest_api.go index ee42dacfc76..ad61d027d06 100644 --- a/usersession/agent/rest_api.go +++ b/usersession/agent/rest_api.go @@ -34,6 +34,8 @@ import ( "github.com/snapcore/snapd/desktop/notification" "github.com/snapcore/snapd/dirs" "github.com/snapcore/snapd/i18n" + "github.com/snapcore/snapd/logger" + "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/strutil" "github.com/snapcore/snapd/systemd" "github.com/snapcore/snapd/usersession/client" @@ -431,6 +433,38 @@ func postPendingRefreshNotification(c *Command, r *http.Request) Response { return SyncResponse(nil) } +func guessAppIcon(si *snap.Info) string { + var icon string + parser := goconfigparser.New() + + // trivial heuristic, if the app is named like a snap then + // it's considered to be the main user facing app and hopefully carries + // a nice icon + mainApp, ok := si.Apps[si.SnapName()] + if ok && !mainApp.IsService() { + // got the main app, grab its desktop file + if err := parser.ReadFile(mainApp.DesktopFile()); err == nil { + icon, _ = parser.Get("Desktop Entry", "Icon") + } + } + if icon != "" { + return icon + } + + // If it doesn't exist, take the first app in the snap with a DesktopFile with icon + for _, app := range si.Apps { + if app.IsService() || app.Name == si.SnapName() { + continue + } + if err := parser.ReadFile(app.DesktopFile()); err == nil { + if icon, err = parser.Get("Desktop Entry", "Icon"); err == nil && icon != "" { + break + } + } + } + return icon +} + func postRefreshFinishedNotification(c *Command, r *http.Request) Response { if ok, resp := validateJSONRequest(r); !ok { return resp @@ -461,10 +495,18 @@ func postRefreshFinishedNotification(c *Command, r *http.Request) Response { notification.WithUrgency(notification.LowUrgency), } + var icon string + if si, err := snap.ReadCurrentInfo(finishRefresh.InstanceName); err == nil { + icon = guessAppIcon(si) + } else { + logger.Noticef("cannot load snap-info for %s: %v", finishRefresh.InstanceName, err) + } + msg := ¬ification.Message{ Title: summary, Body: body, Hints: hints, + Icon: icon, } if err := c.s.notificationMgr.SendNotification(notification.ID(finishRefresh.InstanceName), msg); err != nil { return SyncResponse(&resp{ diff --git a/usersession/agent/rest_api_test.go b/usersession/agent/rest_api_test.go index 6a950240b4f..df47d9746e8 100644 --- a/usersession/agent/rest_api_test.go +++ b/usersession/agent/rest_api_test.go @@ -26,6 +26,7 @@ import ( "fmt" "net/http/httptest" "os" + "path" "path/filepath" "time" @@ -35,6 +36,8 @@ import ( "github.com/snapcore/snapd/desktop/notification" "github.com/snapcore/snapd/desktop/notification/notificationtest" "github.com/snapcore/snapd/dirs" + "github.com/snapcore/snapd/snap" + "github.com/snapcore/snapd/snap/snaptest" "github.com/snapcore/snapd/systemd" "github.com/snapcore/snapd/testutil" "github.com/snapcore/snapd/usersession/agent" @@ -903,3 +906,148 @@ func (s *restSuite) TestPostCloseRefreshNotification(c *C) { "desktop-entry": dbus.MakeVariant("io.snapcraft.SessionAgent"), }) } + +func createDesktopFile(c *C, info *snap.AppInfo, icon string) { + data := []byte("[Desktop Entry]\nName=" + info.Name + "\n") + if icon != "" { + data = append(data, []byte("Icon="+icon+"\n")...) + } + c.Assert(os.MkdirAll(path.Dir(info.DesktopFile()), 0755), IsNil) + c.Assert(os.WriteFile(info.DesktopFile(), data, 0644), IsNil) +} + +func createSnapInfo(snapName string) *snap.Info { + si := snap.Info{ + SideInfo: snap.SideInfo{ + RealName: snapName, + }, + Apps: make(map[string]*snap.AppInfo, 5), + } + return &si +} + +func addAppToSnap(c *C, snapinfo *snap.Info, app string, isService bool, icon string) { + newInfo := snap.AppInfo{ + Snap: snapinfo, + Name: app, + } + if isService { + newInfo.Daemon = "daemon" + } + snapinfo.Apps[app] = &newInfo + createDesktopFile(c, &newInfo, icon) +} + +func (s *restSuite) TestGuessAppIconNoIconPrefixEqualApp(c *C) { + si := createSnapInfo("app1") + addAppToSnap(c, si, "app1", false, "") + icon := agent.GuessAppIcon(si) + c.Check(icon, Equals, "") +} + +func (s *restSuite) TestGuessAppIconNoIconPrefixDifferentApp(c *C) { + si := createSnapInfo("snap1") + addAppToSnap(c, si, "app1", false, "") + icon := agent.GuessAppIcon(si) + c.Check(icon, Equals, "") +} + +func (s *restSuite) TestGuessAppIconPrefixDifferentApp(c *C) { + si := createSnapInfo("snap1") + addAppToSnap(c, si, "app1", false, "iconname") + icon := agent.GuessAppIcon(si) + c.Check(icon, Equals, "iconname") +} + +func (s *restSuite) TestGuessAppIconPrefixEqualApp(c *C) { + si := createSnapInfo("app1") + addAppToSnap(c, si, "app1", false, "iconname1") + addAppToSnap(c, si, "app2", false, "iconname2") + icon := agent.GuessAppIcon(si) + c.Check(icon, Equals, "iconname1") +} + +func (s *restSuite) TestGuessAppIconServicePrefixEqualApp(c *C) { + si := createSnapInfo("app1") + addAppToSnap(c, si, "app1", true, "iconname") + icon := agent.GuessAppIcon(si) + c.Check(icon, Equals, "") +} + +func (s *restSuite) TestGuessAppIconServicePrefixDifferentApp(c *C) { + si := createSnapInfo("snap1") + addAppToSnap(c, si, "app1", true, "iconname") + icon := agent.GuessAppIcon(si) + c.Check(icon, Equals, "") +} + +func (s *restSuite) TestGuessAppIconServiceTwoApps(c *C) { + si := createSnapInfo("app1") + addAppToSnap(c, si, "app1", true, "iconname1") + addAppToSnap(c, si, "app2", false, "iconname2") + icon := agent.GuessAppIcon(si) + c.Check(icon, Equals, "iconname2") +} + +func (s *restSuite) TestGuessAppIconServiceTwoAppsServices(c *C) { + si := createSnapInfo("app1") + addAppToSnap(c, si, "app1", true, "iconname1") + addAppToSnap(c, si, "app2", true, "iconname2") + icon := agent.GuessAppIcon(si) + c.Check(icon, Equals, "") +} + +func (s *restSuite) TestGuessAppIconServiceTwoAppsOneServicePrefixDifferent(c *C) { + si := createSnapInfo("snap1") + addAppToSnap(c, si, "app1", true, "iconname1") + addAppToSnap(c, si, "app2", false, "iconname2") + icon := agent.GuessAppIcon(si) + c.Check(icon, Equals, "iconname2") +} + +func (s *restSuite) TestGuessAppIconTwoAppsPrefixDifferent(c *C) { + si := createSnapInfo("snap1") + addAppToSnap(c, si, "app1", false, "iconname1") + addAppToSnap(c, si, "app2", false, "iconname2") + icon := agent.GuessAppIcon(si) + if (icon != "iconname1") && (icon != "iconname2") { + c.Fail() + } +} + +func (s *restSuite) TestPostCloseRefreshNotificationWithIconDefault(c *C) { + snap.MockSanitizePlugsSlots(func(snapInfo *snap.Info) {}) + // add a notification first + mockYaml := ` +name: snap-name +apps: + other-app: + command: /bin/foo + snap-name: + command: /bin/foo +` + snaptest.MockSnapCurrent(c, mockYaml[1:], &snap.SideInfo{ + Revision: snap.R("42"), + }) + + desktopEntry := ` +[Desktop Entry] +Icon=foo.png +` + os.MkdirAll(dirs.SnapDesktopFilesDir, 0755) + c.Assert(os.WriteFile(filepath.Join(dirs.SnapDesktopFilesDir, "snap-name_snap-name.desktop"), []byte(desktopEntry[1:]), 0644), IsNil) + refreshInfo := &client.FinishedSnapRefreshInfo{InstanceName: "snap-name"} + s.testPostFinishRefreshNotificationBody(c, refreshInfo) + + notifications := s.notify.GetAll() + c.Assert(notifications, HasLen, 1) + n := notifications[0] + c.Check(n.Summary, Equals, `snap-name was updated.`) + c.Check(n.Body, Equals, "Ready to launch.") + c.Check(n.Hints, DeepEquals, map[string]dbus.Variant{ + "urgency": dbus.MakeVariant(byte(notification.LowUrgency)), + "desktop-entry": dbus.MakeVariant("io.snapcraft.SessionAgent"), + }) + // boring stuff is checked above + c.Check(n.Icon, Equals, "foo.png") +}