Skip to content
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.

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

Problems found while migrating CERNBox to 2.13.1 #2787

Closed
diocas opened this issue Jan 10, 2020 · 10 comments
Closed

Problems found while migrating CERNBox to 2.13.1 #2787

diocas opened this issue Jan 10, 2020 · 10 comments

Comments

@diocas
Copy link

diocas commented Jan 10, 2020

Hi,

We're working on the release of 2.13.1 to our CERN community and there were a couple of issues that we found.
Please keep in mind that we have our own implementation of the backend (REVA), which differs from the OC10 server, so some of these issues are not bugs per se but what we see as limitations.
I can open some of them in separate issues if you prefer.

1. App crashes looking for parent

If I have a folder, I would expect dav to return httpd/unix-directory as d:getcontenttype. This is actually something that is present in the new REVA/OCIS (propfind.go#L230).
But, if I return this, the app crashes here with parentPath null.

2. App fails to load shares

Due to our storage infrastructure, where files might live in different storage instances and fileIDs are not shared between them, item_source and file_source have the format InstanceName:FileID, for example home:123456. This works for both the web and sync clients.
But on the mobile app, when getting the shares of a file (GET ocs/v2.php/apps/files_sharing/api/v1/shares), the app gives an error and displays nothing. This happens because it tries to convert file/item source into Long (ShareXMLParser.kt#L246 and ShareXMLParser.kt#L263)

3. App uses the old webdav endpoint

When we released 2.13 into our beta channel, our testers (including me) were updated from the version 2.6.0. Everything seemed to be working fine, until someone tried to install the app in a fresh device (and when I removed my account and added it again).
What happened was that the people who updated still used the old webdav endpoint (/remote.php/webdav) and the people who started clean used the new one (/remote.php/dav/files/), which was not compliant. After discussing this in the OC chat, I was informed that the app should always use the new endpoint and is not compatible with the old one. So this is a Bug that need to be fixed before we release the update to everyone else.

4. App freezes when app switching

If I opened OC, switch to another app and tried to return to OC, the app froze and I had to close it and open it again.
I cannot reproduce this problem anymore, maybe there were some leftovers in the device like the ones reported in 3.. Maybe it was caused by the fact that the app was using the old webdav endpoint, I'm not sure. I'll report back later if I manage to reproduce it again.

Environment data

Android version:
10
Device model:
OnePlus 6T
Stock or customized system:
Stock
ownCloud app version:
2.13.1
ownCloud server version:
CERNBox/REVA

@labkode
Copy link
Member

labkode commented Jan 10, 2020

@michaelstingl @davivel @davigonz would be possible to address these points in a patch release?

For 1, it should be harmless to accept the content type, and I think this is one of the core fields of the WebDAV protocol.

For 2, the desktop sync client and web UI already uses a string instead of a number for the file_source/item_source variables. These type of incosistencies between different implementations of the ownCloud protocol would be solved by using the same SDK across all clients, that's why we are pushing for GRPC and Protocol Buffers to achieve that.

For 3 maybe we need to configure some paremeters on our side like a capability to force the new endpoint, we can provide you our server capabilities if would be useful.

Thanks!

@michaelstingl
Copy link
Contributor

What happened was that the people who updated still used the old webdav endpoint (/remote.php/webdav) and the people who started clean used the new one (/remote.php/dav/files/),

@jesmrec You did research this a while ago? Were you able to reproduce?

@jesmrec
Copy link
Collaborator

jesmrec commented Jan 13, 2020

I updated from 2.6 to 2.13 and i did not experience problems with endpoints, but i will recheck just in case. We will also review the remaining suggestions to check how feasible they are at this moment.

@davigonz
Copy link
Contributor

davigonz commented Jan 13, 2020

Thanks @diocas and @labkode for your feedback.

  1. App crashes looking for parent

This is something related to the oC server, since as you say, it does not seem to return httpd/unix-directory as d:getcontenttype. Of course, the app shouldn't crash under any condition, so you could create an issue in server and as soon as it returns what expected, we will fix the crash in the app. Or just give us an instance of your REVA server so we can reproduce and fix it directly, without waiting the oC server fix.

  1. App fails to load shares

Not only the parser would need to be changed but also data and domain fields, some tests and maybe migrations, not sure, we will check it.

  1. App uses the old webdav endpoint

@jesmrec will recheck this.

  1. App freezes when app switching

Please check #2626 , it might be related

would be possible to address these points in a patch release?

Guess we will discuss it in a coming planning meeting, in which we check our priorities. By the way, @davivel is not part of the team anymore, you could ping @abelgardep (Android dev) or @jesmrec (mobile QA) as well, thanks.

@diocas
Copy link
Author

diocas commented Jan 13, 2020

Hi @davigonz ,

We do not use the OC server, it's our own implementation. So there's nothing to report, I just stopped returning d:getcontenttype in our server and now it works.
I was just confused because the new REVA/OCIS returns that for folders. Probably it wasn't tested yet and it's not supposed to be like that (@butonic)? Or this was necessary for Phoenix?

About the issue you referenced for the app freeze problem, I was only using one account. I was switching between OC (CERNBox branded) and other app, say Gmail, and when returning the app would freeze. This was also reported by one of our beta testers. But I'll report back once we have everything fixed (we still have to deploy the webdav fixes in our server), since I'm not seeing this problem anymore with my test server.

Thanks!

@michaelstingl
Copy link
Contributor

@diocas just FYI, @jesmrec had a closer look a "3. App uses the old webdav endpoint" in #2790

@labkode
Copy link
Member

labkode commented Jan 17, 2020

@jesmrec Thanks for having the time to look into that! Looks like it was a problem related with our environment.

The blocker part is point 2. App fails to load shares because of the parsing, @davigonz do you have any news on that?

@labkode
Copy link
Member

labkode commented Jan 22, 2020

@michaelstingl any news on point 2 we have many users that cannot work because of this issue.

@davigonz
Copy link
Contributor

davigonz commented Jan 22, 2020

do you have any news on that?

@labkode we've been thinking in different solutions:

  1. Remove item_source and file_source params since are not being used in the ownCloud Android app.
  2. Keep both params (just in case are needed in the future) and convert them to String instead of Long so other formats can be used.

We are currently focused on 2.14.1 for customers and 2.14.2, so any of the fixes I mention could be included in 2.15 maybe? What do you think @michaelstingl ?

@jesmrec
Copy link
Collaborator

jesmrec commented Jun 23, 2022

2.13 is nowadays a pretty old one. Please recheck with 2.21 in case any of these problems is still alive.

@jesmrec jesmrec closed this as completed Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants