search

cs3org / wopiserver

A vendor-neutral application gateway compatible with the WOPI specifications.
Apache License 2.0
55 stars 26 forks source link

Reimplement UnlockAndRelock #87

Closed glpatcern closed 2 years ago

glpatcern commented 2 years ago

Following up https://github.com/cs3org/cs3apis/pull/183

Closes #86.

lgtm-com[bot] commented 2 years ago

This pull request introduces 2 alerts when merging 7cb2a3e273bcc83abb0ea3dd78d2dcdefd7467d4 into 7d3c3c0977a117405e9f451feba57d95232ede13 - view on LGTM.com

new alerts:

glpatcern commented 2 years ago

Now it should be properly streamlined: in normal cases either setlock or refreshlock is called, if a race is detected a further refreshlock call may happen. The test suite was also extended to explicitly include the unlockAndRelock case. @micbar and @javfg please have a look.

glpatcern commented 2 years ago

Of course with locking things are never so simple, therefore I propose to keep this PR to the minimal implementation in order to move on and not be blocked, and postpone the refactoring/reordering of the setlock and refreshlock calls to a separate PR.

With this in mind, the Microsoft WOPI validator passes all relevant tests with xrootd as backend:

image

@micbar could you test against CS3 and Reva edge? For now we still don't have a proper support of parent_id in the eosfs storage driver and essentially we can't run wopiserver on top of Reva master and eos.

micbar commented 2 years ago

how do you run the validator in the UI?

glpatcern commented 2 years ago

how do you run the validator in the UI?

This is actually a feature of the Microsoft Cloud, and it's even their recommended way to run the test suite: you need to configure your backend to open .wopitest and .wopitestx files via Office 365. What we have is two custom mimetypes application/wopi-validator and application/wopi-validator-x mapped to those file extensions, in the storage driver config, and we have associated them to the MS Office 365 app provider, in the app provider config as well as in the gateway, so the frontend knows they can be "opened with MS Office 365".

With that, you touch a test.wopitest empty file in an empty folder and you open it in MS Office 365 to get that UI.

micbar commented 2 years ago

With that, you touch a test.wopitest empty file in an empty folder and you open it in MS Office 365 to get that UI.

ok, thanks. That works on my remote instance, but not locally 😄

micbar commented 2 years ago

@glpatcern I can run the tests sucessfully now after fixing a small bug in the DecomposedFS https://github.com/cs3org/reva/pull/3307

I think we are safe to merge now.

Finding and Question

Test group: PutRelativeFile
  Fail: PutRelativeFile.FileNameLongerThan512Chars
    CheckFileInfo, response code: 200 OK
    PutRelativeFile, response code: 306 Unused
      ExceptionValidator: Exception thrown while executing: (WebException)Timeout: The operation has timed out.
      Incorrect StatusCode. Expected: 200, Actual: 306
      Incorrect StatusCode. Expected: 400, Actual: 306
    DeleteFile, response code: 0 0

Our well-known filesystems have a 255 bytes limit on filenames. How does EOS handle it?

glpatcern commented 2 years ago

Our well-known filesystems have a 255 bytes limit on filenames. How does EOS handle it?

In fact EOS keeps the namespace in a dedicated metadata store, and there's no such limit. So we seamlessly pass that test :) image