refactor(graph): use BaseGraphService.publicBaseURL consistently

drive.WebUrl, driveItem.WebUrl and the public share link WebUrl all
derive from the same config value (graph.spaces.webdav_base), but only
driveItem.WebUrl used the pre-parsed BaseGraphService.publicBaseURL.
The other two re-parsed the config on every call.

Add a webURLForResource method on BaseGraphService for the /f/<id> URLs
(used twice, with a *string return matching the libregraph DriveItem
field shape), and inline g.publicBaseURL for the single /s/<token>
share-link case. Convert cs3ResourceToDriveItem and formatDriveItems
from free functions into BaseGraphService methods so they pick up
logger and publicBaseURL from the receiver. This also aligns them with
the surrounding code: BaseGraphService already exposes ~15 similar
methods, so the two free functions were the odd ones out.

Net: all three WebUrls are now constructed from a single pre-parsed
URL, and the (g.logger, g.publicBaseURL) plumbing at 7 call sites
disappears.
This commit is contained in:
Dominik Schmidt committed 2026-09-07 12:59:08 +02:00
1 parent 81536bbd0e
commit 13da9331f2
7 files changed
+30 -43

No files matched your search

@@ -404,7 +404,7 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID
driveItems := make(driveItemsByResourceID, 1)
// we can use the statResponse to build the drive item before fetching the shares
item, err := cs3ResourceToDriveItem(s.logger, s.publicBaseURL, statResponse.GetInfo())
item, err := s.cs3ResourceToDriveItem(statResponse.GetInfo())
if err != nil {
return collectionOfPermissions, err
}
+11 -9
View File
@@ -52,6 +52,15 @@ type BaseGraphService struct {
publicBaseURL *url.URL
}
// webURLForResource returns the public web URL pointing at the given resource
// (e.g. https://cloud.example.com/f/<resource-id>), using the pre-parsed
// publicBaseURL held by the service.
func (g BaseGraphService) webURLForResource(rid *storageprovider.ResourceId) *string {
u := *g.publicBaseURL
u.Path = path.Join(u.Path, "f", storagespace.FormatResourceID(rid))
return libregraph.PtrString(u.String())
}
func (g BaseGraphService) getDriveItem(ctx context.Context, ref *storageprovider.Reference) (*libregraph.DriveItem, error) {
gatewayClient, err := g.gatewaySelector.Next()
if err != nil {
@@ -66,7 +75,7 @@ func (g BaseGraphService) getDriveItem(ctx context.Context, ref *storageprovider
refStr, _ := storagespace.FormatReference(ref)
return nil, fmt.Errorf("could not stat %s: %s", refStr, res.GetStatus().GetMessage())
}
return cs3ResourceToDriveItem(g.logger, g.publicBaseURL, res.GetInfo())
return g.cs3ResourceToDriveItem(res.GetInfo())
}
func (g BaseGraphService) CS3ReceivedSharesToDriveItems(ctx context.Context, receivedShares []*collaboration.ReceivedShare) ([]libregraph.DriveItem, error) {
@@ -217,14 +226,6 @@ func (g BaseGraphService) cs3SpacePermissionsToLibreGraph(ctx context.Context, s
}
func (g BaseGraphService) libreGraphPermissionFromCS3PublicShare(createdLink *link.PublicShare) (*libregraph.Permission, error) {
webURL, err := url.Parse(g.config.Spaces.WebDavBase)
if err != nil {
g.logger.Error().
Err(err).
Str("url", g.config.Spaces.WebDavBase).
Msg("failed to parse webURL base url")
return nil, err
}
lt, actions := linktype.SharingLinkTypeFromCS3Permissions(createdLink.GetPermissions())
perm := libregraph.NewPermission()
perm.Id = libregraph.PtrString(createdLink.GetId().GetOpaqueId())
@@ -235,6 +236,7 @@ func (g BaseGraphService) libreGraphPermissionFromCS3PublicShare(createdLink *li
LibreGraphQuickLink: libregraph.PtrBool(createdLink.GetQuicklink()),
}
perm.LibreGraphPermissionsActions = actions
webURL := *g.publicBaseURL
webURL.Path = path.Join(webURL.Path, "s", createdLink.GetToken())
perm.Link.SetWebUrl(webURL.String())
+9 -13
View File
@@ -29,7 +29,6 @@ import (
"github.com/opencloud-eu/reva/v2/pkg/tags"
"github.com/opencloud-eu/reva/v2/pkg/utils"
"github.com/opencloud-eu/opencloud/pkg/log"
"github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode"
"github.com/opencloud-eu/opencloud/services/graph/pkg/unifiedrole"
"github.com/opencloud-eu/opencloud/services/search/pkg/mapping"
@@ -250,7 +249,7 @@ func (g Graph) GetRootDriveChildren(w http.ResponseWriter, r *http.Request) {
return
}
files, err := formatDriveItems(g.logger, g.publicBaseURL, lRes.GetInfos())
files, err := g.formatDriveItems(lRes.GetInfos())
if err != nil {
g.logger.Error().Err(err).Msg("error encoding response as json")
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
@@ -326,7 +325,7 @@ func (g Graph) GetDriveItem(w http.ResponseWriter, r *http.Request) {
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, res.GetStatus().GetMessage())
return
}
driveItem, err := cs3ResourceToDriveItem(g.logger, g.publicBaseURL, res.GetInfo())
driveItem, err := g.cs3ResourceToDriveItem(res.GetInfo())
if err != nil {
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
return
@@ -430,7 +429,7 @@ func (g Graph) listDriveItemChildren(w http.ResponseWriter, r *http.Request, dri
return nil, false
}
files, err := formatDriveItems(g.logger, g.publicBaseURL, res.GetInfos())
files, err := g.formatDriveItems(res.GetInfos())
if err != nil {
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
return nil, false
@@ -483,10 +482,10 @@ func (g Graph) getRemoteItem(ctx context.Context, root *storageprovider.Resource
return item, nil
}
func formatDriveItems(logger *log.Logger, publicBaseURL *url.URL, mds []*storageprovider.ResourceInfo) ([]libregraph.DriveItem, error) {
func (g BaseGraphService) formatDriveItems(mds []*storageprovider.ResourceInfo) ([]libregraph.DriveItem, error) {
responses := make([]libregraph.DriveItem, 0, len(mds))
for i := range mds {
res, err := cs3ResourceToDriveItem(logger, publicBaseURL, mds[i])
res, err := g.cs3ResourceToDriveItem(mds[i])
if err != nil {
return nil, err
}
@@ -500,19 +499,16 @@ func cs3TimestampToTime(t *types.Timestamp) time.Time {
return time.Unix(int64(t.GetSeconds()), int64(t.GetNanos()))
}
func cs3ResourceToDriveItem(logger *log.Logger, publicBaseURL *url.URL, res *storageprovider.ResourceInfo) (*libregraph.DriveItem, error) {
func (g BaseGraphService) cs3ResourceToDriveItem(res *storageprovider.ResourceInfo) (*libregraph.DriveItem, error) {
size := new(int64)
*size = int64(res.GetSize()) // TODO lurking overflow: make size of libregraph drive item use uint64
driveItem := &libregraph.DriveItem{
Id: libregraph.PtrString(storagespace.FormatResourceID(res.GetId())),
Size: size,
Id: libregraph.PtrString(storagespace.FormatResourceID(res.GetId())),
Size: size,
WebUrl: g.webURLForResource(res.GetId()),
}
webURL := *publicBaseURL
webURL.Path = path.Join(webURL.Path, "f", storagespace.FormatResourceID(res.GetId()))
driveItem.WebUrl = libregraph.PtrString(webURL.String())
if name := path.Base(res.GetPath()); name != "" {
driveItem.Name = &name
}
@@ -26,7 +26,8 @@ func TestCS3ResourceToDriveItemPopulatesWebUrl(t *testing.T) {
base, err := url.Parse("https://example.com")
require.NoError(t, err)
item, err := cs3ResourceToDriveItem(&logger, base, res)
g := BaseGraphService{logger: &logger, publicBaseURL: base}
item, err := g.cs3ResourceToDriveItem(res)
require.NoError(t, err)
require.NotNil(t, item.WebUrl)
assert.Equal(t, "https://example.com/f/storage-1$space-1%21item-1", *item.WebUrl)
@@ -36,7 +37,8 @@ func TestCS3ResourceToDriveItemPopulatesWebUrl(t *testing.T) {
base, err := url.Parse("https://example.com/cloud")
require.NoError(t, err)
item, err := cs3ResourceToDriveItem(&logger, base, res)
g := BaseGraphService{logger: &logger, publicBaseURL: base}
item, err := g.cs3ResourceToDriveItem(res)
require.NoError(t, err)
require.NotNil(t, item.WebUrl)
assert.Equal(t, "https://example.com/cloud/f/storage-1$space-1%21item-1", *item.WebUrl)
+1 -11
View File
@@ -862,17 +862,7 @@ func (g Graph) cs3StorageSpaceToDrive(ctx context.Context, baseURL *url.URL, spa
drive.Root.WebDavUrl = libregraph.PtrString(webDavURL.String())
}
webURL, err := url.Parse(g.config.Spaces.WebDavBase)
if err != nil {
logger.Error().
Err(err).
Str("url", g.config.Spaces.WebDavBase).
Msg("failed to parse webURL base url")
return nil, err
}
webURL.Path = path.Join(webURL.Path, "f", storagespace.FormatResourceID(spaceRid))
drive.WebUrl = libregraph.PtrString(webURL.String())
drive.WebUrl = g.webURLForResource(spaceRid)
if space.Owner != nil && space.Owner.Id != nil {
drive.Owner = &libregraph.IdentitySet{
+1 -1
View File
@@ -98,7 +98,7 @@ func (g Graph) FollowDriveItem(w http.ResponseWriter, r *http.Request) {
}
}
driveItem, err := cs3ResourceToDriveItem(g.logger, g.publicBaseURL, statRes.GetInfo())
driveItem, err := g.cs3ResourceToDriveItem(statRes.GetInfo())
if err != nil {
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
return
+3 -6
View File
@@ -96,13 +96,10 @@ func (g Graph) publishEvent(ctx context.Context, ev any) {
}
}
func (g Graph) getWebDavBaseURL() (*url.URL, error) {
webDavBaseURL, err := url.Parse(g.config.Spaces.WebDavBase)
if err != nil {
return nil, err
}
func (g BaseGraphService) getWebDavBaseURL() (*url.URL, error) {
webDavBaseURL := *g.publicBaseURL
webDavBaseURL.Path = path.Join(webDavBaseURL.Path, g.config.Spaces.WebDavPath)
return webDavBaseURL, nil
return &webDavBaseURL, nil
}
// ListResponse is used for proper marshalling of Graph list responses