Merge branch 'master' into fix-forceformat-directplay

server/subsonic/e2e/subsonic_sharing_test.go

@@ -205,3 +205,76 @@ var _ = Describe("Sharing Cross-User Isolation", Ordered, func() {
 		Expect(check.Shares.Share[0].ID).To(Equal(shareID))
 	})
 })
+
+var _ = Describe("Sharing Downloadable Default", func() {
+	var albumID string
+
+	BeforeEach(func() {
+		conf.Server.EnableSharing = true
+		setupTestDB()
+		conf.Server.EnableDownloads = true
+		albumID = albumIDByName("Abbey Road")
+	})
+
+	createShare := func(params ...string) *model.Share {
+		GinkgoHelper()
+		resp := doReq("createShare", append([]string{"id", albumID}, params...)...)
+		Expect(resp.Status).To(Equal(responses.StatusOK))
+		Expect(resp.Shares.Share).To(HaveLen(1))
+		share, err := ds.Share(ctx).Get(resp.Shares.Share[0].ID)
+		Expect(err).ToNot(HaveOccurred())
+		return share
+	}
+
+	DescribeTable("createShare resolves downloadable",
+		func(defaultDownloadable, enableDownloads bool, params []string, expected bool) {
+			conf.Server.DefaultDownloadableShare = defaultDownloadable
+			conf.Server.EnableDownloads = enableDownloads
+
+			Expect(createShare(params...).Downloadable).To(Equal(expected))
+		},
+		Entry("applies the default when the param is absent", true, true, nil, true),
+		Entry("stays off when the default is off", false, true, nil, false),
+		Entry("ignores the default when downloads are disabled", true, false, nil, false),
+		Entry("honors an explicit false over the default", true, true, []string{"downloadable", "false"}, false),
+		Entry("honors an explicit true over the default", false, true, []string{"downloadable", "true"}, true),
+	)
+
+	It("updateShare keeps the current downloadable when the param is absent", func() {
+		conf.Server.DefaultDownloadableShare = true
+		share := createShare()
+		Expect(share.Downloadable).To(BeTrue())
+
+		resp := doReq("updateShare", "id", share.ID, "description", "Updated")
+		Expect(resp.Status).To(Equal(responses.StatusOK))
+
+		updated, err := ds.Share(ctx).Get(share.ID)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(updated.Description).To(Equal("Updated"))
+		Expect(updated.Downloadable).To(BeTrue())
+	})
+
+	It("updateShare applies an explicit downloadable and keeps the description", func() {
+		conf.Server.DefaultDownloadableShare = true
+		share := createShare("description", "Keep me")
+
+		resp := doReq("updateShare", "id", share.ID, "downloadable", "false")
+		Expect(resp.Status).To(Equal(responses.StatusOK))
+
+		updated, err := ds.Share(ctx).Get(share.ID)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(updated.Downloadable).To(BeFalse())
+		Expect(updated.Description).To(Equal("Keep me"))
+	})
+
+	It("updateShare clears the description when it is sent empty", func() {
+		share := createShare("description", "Clear me")
+
+		resp := doReq("updateShare", "id", share.ID, "description", "")
+		Expect(resp.Status).To(Equal(responses.StatusOK))
+
+		updated, err := ds.Share(ctx).Get(share.ID)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(updated.Description).To(BeEmpty())
+	})
+})

server/subsonic/sharing.go

@@ -1,11 +1,13 @@
 package subsonic
 
 import (
+	"cmp"
 	"net/http"
 	"strings"
 	"time"
 
 	"github.com/deluan/rest"
+	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/server/public"
 	"github.com/navidrome/navidrome/server/subsonic/responses"
@@ -60,9 +62,10 @@ func (api *Router) CreateShare(r *http.Request) (*responses.Subsonic, error) {
 	description, _ := p.String("description")
 	repo := api.share.NewRepository(r.Context())
 	share := &model.Share{
-		Description: description,
-		ExpiresAt:   new(p.TimeOr("expires", time.Time{})),
-		ResourceIDs: strings.Join(ids, ","),
+		Description:  description,
+		Downloadable: p.BoolOr("downloadable", conf.Server.DefaultDownloadableShare && conf.Server.EnableDownloads),
+		ExpiresAt:    new(p.TimeOr("expires", time.Time{})),
+		ResourceIDs:  strings.Join(ids, ","),
 	}
 
 	id, err := repo.(rest.Persistable).Save(share)
@@ -87,12 +90,27 @@ func (api *Router) UpdateShare(r *http.Request) (*responses.Subsonic, error) {
 		return nil, err
 	}
 
-	description, _ := p.String("description")
 	repo := api.share.NewRepository(r.Context())
+
+	// The update always writes description and downloadable, so read back the
+	// stored value for whichever one the client omitted.
+	description := p.StringPtr("description")
+	downloadable := p.BoolPtr("downloadable")
+	if description == nil || downloadable == nil {
+		current, err := repo.Read(id)
+		if err != nil {
+			return nil, err
+		}
+		cur := current.(*model.Share)
+		description = cmp.Or(description, &cur.Description)
+		downloadable = cmp.Or(downloadable, &cur.Downloadable)
+	}
+
 	share := &model.Share{
-		ID:          id,
-		Description: description,
-		ExpiresAt:   new(p.TimeOr("expires", time.Time{})),
+		ID:           id,
+		Description:  *description,
+		Downloadable: *downloadable,
+		ExpiresAt:    new(p.TimeOr("expires", time.Time{})),
 	}
 
 	err = repo.(rest.Persistable).Update(id, share)