Repository: owncloud/ocis
Pull request #10335 (2024-10-17 12:33:29)

feat: include a provider option to allow custom app names
<!--
Thanks for submitting a change to ownCloud!

This is the bug tracker for the oCIS component. Find other components at https://github.com/owncloud/core/blob/master/.github/CONTRIBUTING.md#guidelines

For fixing potential security issues please see https://owncloud.org/security/

To make it possible for us to get your change reviewed and merged please carefully fill out the requested information below.

Please note that any kind of change needs first be submitted to the master branch which holds the next version of oCIS.

Please set the following labels:

- Set label "Status:Needs-Review" for review or "Status:In-Progress" if the PR still has open tasks.
- Assignment: assign to self
- Reviewers: pick at least one
-->

## Description
Introduce the `Provider` option to handle some differences between web office products. This will be used instead of the `Name`. Note that the `Name` will still be used in other places such as service names, and it will be shown in the web interface.

The intention is to provide a bit of extra branding: you could have a specific Collabora installation fully customized and branded to your liking, so you could use `BBox` as `Name` and keep `Collabora` as `Provider`.

**Note**: There are some issues with the naming used as `Name`, so it's highly recommended to use only alphanumeric chars. Spaces are known to cause issues.

## Related Issue
https://github.com/owncloud/ocis/issues/10306

## Motivation and Context
We need to distinguish between the name and the provider.

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->
- test environment:
- test case 1:
- test case 2:
- ...

## Screenshots (if appropriate):

## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Technical debt
- [ ] Tests only (no source changes)

## Checklist:
<!-- Tick the checkboxes when done. -->
<!-- Raise documentation ticket in owncloud.github.io/ -->
- [ ] Code changes
- [ ] Unit tests added
- [ ] Acceptance tests added
- [ ] Documentation ticket raised: <link> 

## Notes
Need to consider a migration, otherwise it could be a breaking change.
For installations with Collabora, this change shouldn't cause any issue because the default provider will also be Collabora. However, for other installations, the provider will also be Collabora despite connecting to an OnlyOffice installation (for example). This might cause some features to break.

We also need changes in reva for the new templates. It's currently using the app name (which might be different) instead of the provider name. Right now, the collaboration service can't sent the provider name to reva.

update-docs[bot]: Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a [changelog](https://github.com/owncloud/ocis/blob/master/changelog/README.md) item based on your changes.

sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=owncloud_ocis&pullRequest=10335) **Quality Gate passed**  
Issues  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=10335&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=10335&issueStatuses=ACCEPTED)

Measures  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=owncloud_ocis&pullRequest=10335&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=10335&metric=new_coverage&view=list)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=10335&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=owncloud_ocis&pullRequest=10335)


butonic: Too many things are called provider, already. Can we use `Product`? IMO that better conveys product specific peculiarities. I don't think being able to deploy multiple versions of the same product makes sense, not even in demo installations so we don't need a generic provider property. If we really want to distinguish providers we can introduce an `ID` property, but for now I prefer to only add `Product`.
jvillafanez: Changed `Provider` to `Product`.

> I don't think being able to deploy multiple versions of the same product makes sense, not even in demo installations

You could have OnlyOffice 7 running and you could try / test the integration with OnlyOffice 8. Having both running at the same time would allow you to switch to the old version if the new one is having problems.
Allowing your users to use different versions would allow you to gather feedback on the new version, wait for critical bugs (at least for your users) to be fixed in the new version, or even give your users time to adapt to the new version before switching down the old one.
While it might not make sense for small version upgrades (we don't expect big changes for the users), it seems reasonable for mayor version upgrades which could bring more breaking changes.

In theory, having 2 wopi containers with app names "OnlyOffice7" and "OnlyOffice8" pointing to their corresponding OnlyOffice servers should work. Unless we want to officially support this setup, it isn't probably worthy to test it, but I'd rather not to block this option if possible.
butonic: Yes, I see that use case as well. But it would still be tha same app product: `onlyofffice`. The  app name would be different `OnlyOffice 7` and `OnlyOffice 8`. I am voting to degrade app name to be just a displayname, which is currently not the case. IIRC the appregistry relies on the app name to find the right proider. Which I think still works when we treat it as a display name. But the collaboration service as an app provider needs to be able to take different codepaths based on the product.

Hm 🤔 

the collaboration service is its own wopi service and is always tied to a product. We could make this a service config option. Then we would not need to change the CS3 api.

Right? What am I missing?
jvillafanez: In theory, the PR should work in that scenario once it's finished.

In any case, I think we need reva to be aware of the product due to how the office templates are implemented there. Using the name won't be enough because it could be any name for any known product.
I don't know what are the planned changes to "de-hardcode" the product names in reva, but I don't see a way to send that information right now with the current CS3 API.
sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=owncloud_ocis&pullRequest=10335) **Quality Gate passed**  
Issues  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=10335&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=10335&issueStatuses=ACCEPTED)

Measures  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=owncloud_ocis&pullRequest=10335&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=10335&metric=new_coverage&view=list)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=10335&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=owncloud_ocis&pullRequest=10335)



Patch:
diff --git a/.drone.star b/.drone.star
index 2764e87072f..458c2a245ef 100644
--- a/.drone.star
+++ b/.drone.star
@@ -2935,14 +2935,17 @@ def wopiCollaborationService(name):
 
     if name == "collabora":
         environment["COLLABORATION_APP_NAME"] = "Collabora"
+        environment["COLLABORATION_APP_PRODUCT"] = "Collabora"
         environment["COLLABORATION_APP_ADDR"] = "https://collabora:9980"
         environment["COLLABORATION_APP_ICON"] = "https://collabora:9980/favicon.ico"
     elif name == "onlyoffice":
         environment["COLLABORATION_APP_NAME"] = "OnlyOffice"
+        environment["COLLABORATION_APP_PRODUCT"] = "OnlyOffice"
         environment["COLLABORATION_APP_ADDR"] = "https://onlyoffice"
         environment["COLLABORATION_APP_ICON"] = "https://onlyoffice/web-apps/apps/documenteditor/main/resources/img/favicon.ico"
     elif name == "fakeoffice":
         environment["COLLABORATION_APP_NAME"] = "FakeOffice"
+        environment["COLLABORATION_APP_PRODUCT"] = "Microsoft"
         environment["COLLABORATION_APP_ADDR"] = "http://fakeoffice:8080"
 
     environment["COLLABORATION_WOPI_SRC"] = "http://%s:9300" % service_name
diff --git a/services/collaboration/pkg/config/app.go b/services/collaboration/pkg/config/app.go
index a2b2f7bb118..d22e0d43c57 100644
--- a/services/collaboration/pkg/config/app.go
+++ b/services/collaboration/pkg/config/app.go
@@ -2,7 +2,8 @@ package config
 
 // App defines the available app configuration.
 type App struct {
-	Name        string `yaml:"name" env:"COLLABORATION_APP_NAME" desc:"The name of the app, either Collabora, OnlyOffice, Microsoft365 or MicrosoftOfficeOnline" introductionVersion:"6.0.0"`
+	Name        string `yaml:"name" env:"COLLABORATION_APP_NAME" desc:"The name of the app" introductionVersion:"6.0.0"`
+	Product     string `yaml:"product" env:"COLLABORATION_APP_PRODUCT" desc:"The WebOffice app, either Collabora, OnlyOffice, Microsoft365 or MicrosoftOfficeOnline" introductionVersion:"%%NEXT%%"`
 	Description string `yaml:"description" env:"COLLABORATION_APP_DESCRIPTION" desc:"App description" introductionVersion:"6.0.0"`
 	Icon        string `yaml:"icon" env:"COLLABORATION_APP_ICON" desc:"Icon for the app" introductionVersion:"6.0.0"`
 	LockName    string `yaml:"lockname" env:"COLLABORATION_APP_LOCKNAME" desc:"Name for the app lock" introductionVersion:"6.0.0"`
diff --git a/services/collaboration/pkg/config/defaults/defaultconfig.go b/services/collaboration/pkg/config/defaults/defaultconfig.go
index d8bc4aab3c4..8287fcd00f8 100644
--- a/services/collaboration/pkg/config/defaults/defaultconfig.go
+++ b/services/collaboration/pkg/config/defaults/defaultconfig.go
@@ -24,6 +24,7 @@ func DefaultConfig() *config.Config {
 		},
 		App: config.App{
 			Name:        "Collabora",
+			Product:     "Collabora",
 			Description: "Open office documents with Collabora",
 			Icon:        "image-edit",
 			LockName:    "com.github.owncloud.collaboration",
diff --git a/services/collaboration/pkg/connector/fileconnector.go b/services/collaboration/pkg/connector/fileconnector.go
index b8dc73382d5..073a18daf1f 100644
--- a/services/collaboration/pkg/connector/fileconnector.go
+++ b/services/collaboration/pkg/connector/fileconnector.go
@@ -1172,7 +1172,7 @@ func (f *FileConnector) CheckFileInfo(ctx context.Context) (*ConnectorResponse,
 	// This will help with the CI because we're using a "FakeOffice" app
 	// for the wopi validator, which requires a Microsoft fileinfo
 	var info fileinfo.FileInfo
-	switch strings.ToLower(f.cfg.App.Name) {
+	switch strings.ToLower(f.cfg.App.Product) {
 	case "collabora":
 		info = &fileinfo.Collabora{}
 	case "onlyoffice":
diff --git a/services/collaboration/pkg/helpers/registration.go b/services/collaboration/pkg/helpers/registration.go
index 0fa416bd4be..a6609fe20e0 100644
--- a/services/collaboration/pkg/helpers/registration.go
+++ b/services/collaboration/pkg/helpers/registration.go
@@ -68,6 +68,7 @@ func RegisterAppProvider(
 			Icon:        cfg.App.Icon,
 			Address:     cfg.GRPC.Namespace + "." + cfg.Service.Name + "." + cfg.App.Name,
 			MimeTypes:   mimeTypes,
+			ProductName: cfg.App.Product,
 		},
 	}
 	gwc, err := gws.Next()
diff --git a/services/collaboration/pkg/service/grpc/v0/service.go b/services/collaboration/pkg/service/grpc/v0/service.go
index 5fa63b9ab63..74e12be1307 100644
--- a/services/collaboration/pkg/service/grpc/v0/service.go
+++ b/services/collaboration/pkg/service/grpc/v0/service.go
@@ -188,7 +188,7 @@ func (s *Service) getAppUrl(fileExt string, viewMode appproviderv1beta1.ViewMode
 	// prioritize view action if possible
 	appURL := s.getAppUrlFor("view", fileExt)
 
-	if strings.ToLower(s.config.App.Name) == "collabora" {
+	if strings.ToLower(s.config.App.Product) == "collabora" {
 		// collabora provides only one action per extension. usual options
 		// are "view" (checked above), "edit" or "view_comment" (this last one
 		// is exclusive of collabora)
@@ -276,7 +276,7 @@ func (s *Service) addQueryToURL(baseURL string, req *appproviderv1beta1.OpenInAp
 	}
 
 	if lang != "" {
-		switch strings.ToLower(s.config.App.Name) {
+		switch strings.ToLower(s.config.App.Product) {
 		case "collabora":
 			q.Add("lang", lang)
 		case "onlyoffice":


Test patch:
diff --git a/services/collaboration/pkg/connector/fileconnector_test.go b/services/collaboration/pkg/connector/fileconnector_test.go
index f03ab5044b4..b0f48586f59 100644
--- a/services/collaboration/pkg/connector/fileconnector_test.go
+++ b/services/collaboration/pkg/connector/fileconnector_test.go
@@ -48,6 +48,7 @@ var _ = Describe("FileConnector", func() {
 			App: config.App{
 				LockName: "testName_for_unittests", // Only the LockName is used
 				Name:     "test",
+				Product:  "Microsoft",
 			},
 			Wopi: config.Wopi{
 				WopiSrc: "https://ocis.server.prv",
@@ -1761,6 +1762,7 @@ var _ = Describe("FileConnector", func() {
 
 			// change wopi app provider
 			cfg.App.Name = "Collabora"
+			cfg.App.Product = "Collabora"
 
 			expectedFileInfo := &fileinfo.Collabora{
 				OwnerID:                 "61616262636340637573746f6d496470", // hex of aabbcc@customIdp
@@ -1834,6 +1836,7 @@ var _ = Describe("FileConnector", func() {
 
 			// change wopi app provider
 			cfg.App.Name = "Collabora"
+			cfg.App.Product = "Collabora"
 
 			expectedFileInfo := &fileinfo.Collabora{
 				OwnerID:                 "61616262636340637573746f6d496470", // hex of aabbcc@customIdp
@@ -1921,6 +1924,7 @@ var _ = Describe("FileConnector", func() {
 
 			// change wopi app provider
 			cfg.App.Name = "OnlyOffice"
+			cfg.App.Product = "OnlyOffice"
 
 			response, err := fc.CheckFileInfo(ctx)
 			Expect(err).ToNot(HaveOccurred())
diff --git a/services/collaboration/pkg/service/grpc/v0/service_test.go b/services/collaboration/pkg/service/grpc/v0/service_test.go
index 636a9f4acfa..1c2359ad856 100644
--- a/services/collaboration/pkg/service/grpc/v0/service_test.go
+++ b/services/collaboration/pkg/service/grpc/v0/service_test.go
@@ -147,6 +147,7 @@ var _ = Describe("Discovery", func() {
 				cfg.Wopi.Secret = "my_supa_secret"
 				cfg.Wopi.DisableChat = disableChat
 				cfg.App.Name = appName
+				cfg.App.Product = appName
 
 				myself := &userv1beta1.User{
 					Id: &userv1beta1.UserId{
@@ -333,6 +334,7 @@ var _ = Describe("Discovery", func() {
 			cfg.Wopi.WopiSrc = "htttps://wopiserver.test.prv"
 			cfg.Wopi.Secret = "my_supa_secret"
 			cfg.App.Name = "OnlyOffice"
+			cfg.App.Product = "OnlyOffice"
 
 			myself := &userv1beta1.User{
 				Id: &userv1beta1.UserId{