Skip to content

fix: flaky wopi test due to app registry ordering #540

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.

Sign up for GitHub

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

Open
mklos-kw wants to merge 1 commit into
base: main
Choose a base branch
from
+46 −28
Open

fix: flaky wopi test due to app registry ordering #540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions changelog/unreleased/fix-wopi-app-open-with-web-due-to-app-registry-ordering.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Bugfix: Fix WOPI open-with-web due to app registry ordering

Made app selection deterministic when no app is specified: the system now uses the configured default app for the file type, or picks the first provider sorted by priority then name, ensuring users get consistent results instead of random app selection based on registration timing.

https://github.com/owncloud/reva/pull/540
38 changes: 23 additions & 15 deletions internal/http/services/appprovider/appprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"strings"

appregistry "github.com/cs3org/go-cs3apis/cs3/app/registry/v1beta1"
"github.com/owncloud/reva/v2/pkg/app/registry/micro"
providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1"
gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
Expand Down Expand Up @@ -403,23 +404,30 @@ func (s *svc) handleOpen(openMode int) http.HandlerFunc {

appName := r.Form.Get("app_name")
if appName == "" {
apps, err := client.GetAppProviders(ctx, &appregistry.GetAppProvidersRequest{
ResourceInfo: statRes.Info,
defRes, err := client.GetDefaultAppProviderForMimeType(ctx, &appregistry.GetDefaultAppProviderForMimeTypeRequest{
MimeType: statRes.Info.MimeType,
})
if err != nil {
writeError(w, r, appErrorServerError, "error getting app providers", err)
return
}
if apps.Status.Code != rpc.Code_CODE_OK {
writeError(w, r, appErrorServerError, "error getting app providers", nil)
return
}
if len(apps.Providers) == 0 {
writeError(w, r, appErrorProviderNotFound, "no app providers found", nil)
return
if err == nil && defRes.Status.Code == rpc.Code_CODE_OK && defRes.Provider != nil {
appName = defRes.Provider.Name
} else {
apps, err := client.GetAppProviders(ctx, &appregistry.GetAppProvidersRequest{
ResourceInfo: statRes.Info,
})
if err != nil {
writeError(w, r, appErrorServerError, "error getting app providers", err)
return
}
if apps.Status.Code != rpc.Code_CODE_OK {
writeError(w, r, appErrorServerError, "error getting app providers", nil)
return
}
if len(apps.Providers) == 0 {
writeError(w, r, appErrorProviderNotFound, "no app providers found", nil)
return
}
Comment on lines +413 to +427
Copy link
Collaborator

@kobergj kobergj Feb 19, 2026

Choose a reason for hiding this comment

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

Why do we still need this logic here? Shouldn't GetDefaultAppProviderForMimeType already resolve that for us?

Copy link
Member Author

@mklos-kw mklos-kw Feb 19, 2026

Choose a reason for hiding this comment

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

apps.Providers[0] would be a last resort fallback - reva support priority config. Can be removed if can be a source of further errors.

Copy link
Member

@jvillafanez jvillafanez Feb 19, 2026

Choose a reason for hiding this comment

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

I think we'd need to do a SetDefaultAppProviderForMimeType call somewhere. We can't set a default from the AddAppProvider call.
This needs to be setup by the admin, likely via environment variables (not done at the moment). And we need to assume that the admin won't do it.

Copy link
Member Author

@mklos-kw mklos-kw Feb 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

ocis test FakeOffice config: src

Added stable sorting here in handler and in registry that should be enough to fix test issue.
Good point, for no-config (quickstart) case there could be some auto register for default app - I'd take into a separate issue.

micro.SortByPriorityThenName(apps.Providers)
appName = apps.Providers[0].Name
}

appName = apps.Providers[0].Name
}

openReq := gateway.OpenInAppRequest{
Expand Down
29 changes: 17 additions & 12 deletions pkg/app/registry/micro/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ package micro

import (
"context"
"sort"
"strconv"
"sync"
"time"

"sort"
"strconv"
"strings"

registrypb "github.com/cs3org/go-cs3apis/cs3/app/registry/v1beta1"
typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
"github.com/owncloud/reva/v2/pkg/app"
Expand Down Expand Up @@ -222,7 +224,7 @@ func (m *manager) updateProvidersFromMicroRegistry() error {
if err != nil {
return err
}
sortByPriority(lst)
SortByPriorityThenName(lst)
for _, outer := range lst {
for _, inner := range outer.MimeTypes {
ma[inner] = append(ma[inner], outer)
Expand All @@ -245,7 +247,8 @@ func equalsProviderInfo(p1, p2 *registrypb.ProviderInfo) bool {
return false
}

func getPriority(p *registrypb.ProviderInfo) string {
// GetPriority extracts priority from ProviderInfo.Opaque, returns defaultPriority ("0") if not set.
func GetPriority(p *registrypb.ProviderInfo) string {
if p.Opaque != nil && len(p.Opaque.Map) != 0 {
if priority, ok := p.Opaque.Map["priority"]; ok {
return string(priority.GetValue())
Expand All @@ -254,12 +257,14 @@ func getPriority(p *registrypb.ProviderInfo) string {
return defaultPriority
}

func sortByPriority(providers []*registrypb.ProviderInfo) {
less := func(i, j int) bool {
prioI, _ := strconv.ParseInt(getPriority(providers[i]), 10, 64)
prioJ, _ := strconv.ParseInt(getPriority(providers[j]), 10, 64)
return prioI < prioJ
}

sort.Slice(providers, less)
// SortByPriorityThenName sorts providers by priority (lower first), then by Name. Used by micro registry and HTTP appprovider.
func SortByPriorityThenName(providers []*registrypb.ProviderInfo) {
sort.Slice(providers, func(i, j int) bool {
pi, _ := strconv.ParseInt(GetPriority(providers[i]), 10, 64)
pj, _ := strconv.ParseInt(GetPriority(providers[j]), 10, 64)
if pi != pj {
return pi < pj
}
return strings.ToLower(providers[i].Name) < strings.ToLower(providers[j].Name)
})
}
2 changes: 1 addition & 1 deletion pkg/app/registry/micro/micro_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -997,7 +997,7 @@ func registerWithMicroReg(ns string, p *registrypb.ProviderInfo) error {
node.Metadata[ns+".app-provider.icon"] = p.Icon

node.Metadata[ns+".app-provider.allow_creation"] = registrypb.ProviderInfo_Capability_name[int32(p.Capability)]
node.Metadata[ns+".app-provider.priority"] = getPriority(p)
node.Metadata[ns+".app-provider.priority"] = GetPriority(p)
if p.DesktopOnly {
node.Metadata[ns+".app-provider.desktop_only"] = "true"
}
Expand Down