owncloud / ocis-reva

:arrows_counterclockwise: reva integration for oCIS
https://owncloud.github.io/extensions/ocis_reva/
Apache License 2.0
4 stars 5 forks source link

Public link share strays side effects #439

Closed PVince81 closed 4 years ago

PVince81 commented 4 years ago

Steps

  1. Check out https://github.com/owncloud/ocis/pull/409
  2. Run the Phoenix test "tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature:82" at least twice

Expected

All passing

Actual

Passes the first time, fails every subsequent times

Tasks

@individual-it @refs

PVince81 commented 4 years ago

More data here:

  1. Cleant up ocis data and restarted ocis
  2. Ran xxx
ocis-accounts
├── accounts
│   ├── 4c510ada-c86b-4815-8820-42cdf82c3d51
│   ├── 820ba2a1-3f54-4538-80a4-2d73007e30bf
│   ├── 932b4540-8d16-481e-8ef4-588e4b6b151c
│   ├── bc596f3c-c955-4328-80a0-60d018b4ad57
│   └── f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c
├── groups
│   ├── 167cbee2-0518-455a-bfb2-031fe0621e5d
│   ├── 262982c1-2362-4afa-bfdf-8cbfef64a06e
│   ├── 34f38767-c937-4eb6-b847-1c175829a2a0
│   ├── 509a9dcd-bb37-4f4f-a01a-19dca27d9cfa
│   ├── 6040aa17-9c64-4fef-9bd0-77234d71bad0
│   ├── 7b87fd49-286e-4a5f-bafd-c535d5dd997a
│   ├── a1726108-01f8-4c30-88df-2b1a9d1cba1a
│   ├── cedc21aa-4072-4614-8676-fa9165f598ff
│   └── dd58e5ec-842e-498b-8800-61f2ec6f911f
└── index.bleve
    ├── index_meta.json
    └── store
ocis-store
├── databases
└── index.bleve
    ├── index_meta.json
    └── store
reva
├── chunks
├── data
├── publicshares
├── shares.json
├── tmp
└── uploadinfo

publicshares contents:

{
  "MJnfTEmnaDawSWh": {
    "password": "",
    "share": "{\"id\":{\"opaqueId\":\"MJnfTEmnaDawSWh\"},\"token\":\"FByPSuDOtImhAtE\",\"resourceId\":{\"storageId\":\"1284d238-aa92-42ce-bdc4-0b0000009154\",\"opaqueId\":\"f1651932-67dc-4874-8604-7bdaa6993111\"},\"permissions\":{\"permissions\":{\"createContainer\":true}},\"owner\":{\"idp\":\"https://localhost:9200\",\"opaqueId\":\"user1\"},\"creator\":{\"idp\":\"https://localhost:9200\",\"opaqueId\":\"user1\"},\"ctime\":{\"seconds\":\"1597323302\",\"nanos\":826749412},\"mtime\":{\"seconds\":\"1597323302\",\"nanos\":826749412}}"
  }
}
  1. Then rerun the tests
    ocis-accounts
    ├── accounts
    │   ├── 4c510ada-c86b-4815-8820-42cdf82c3d51
    │   ├── 820ba2a1-3f54-4538-80a4-2d73007e30bf
    │   ├── 932b4540-8d16-481e-8ef4-588e4b6b151c
    │   ├── bc596f3c-c955-4328-80a0-60d018b4ad57
    │   └── f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c
    ├── groups
    │   ├── 167cbee2-0518-455a-bfb2-031fe0621e5d
    │   ├── 262982c1-2362-4afa-bfdf-8cbfef64a06e
    │   ├── 34f38767-c937-4eb6-b847-1c175829a2a0
    │   ├── 509a9dcd-bb37-4f4f-a01a-19dca27d9cfa
    │   ├── 6040aa17-9c64-4fef-9bd0-77234d71bad0
    │   ├── 7b87fd49-286e-4a5f-bafd-c535d5dd997a
    │   ├── a1726108-01f8-4c30-88df-2b1a9d1cba1a
    │   ├── cedc21aa-4072-4614-8676-fa9165f598ff
    │   └── dd58e5ec-842e-498b-8800-61f2ec6f911f
    └── index.bleve
    ├── index_meta.json
    └── store
    ocis-store
    ├── databases
    └── index.bleve
    ├── index_meta.json
    └── store
    reva
    ├── chunks
    ├── data
    ├── publicshares
    ├── shares.json
    ├── tmp
    └── uploadinfo

publicshares:

{
  "MJnfTEmnaDawSWh": {
    "password": "",
    "share": "{\"id\":{\"opaqueId\":\"MJnfTEmnaDawSWh\"},\"token\":\"FByPSuDOtImhAtE\",\"resourceId\":{\"storageId\":\"1284d238-aa92-42ce-bdc4-0b0000009154\",\"opaqueId\":\"f1651932-67dc-4874-8604-7bdaa6993111\"},\"permissions\":{\"permissions\":{\"createContainer\":true}},\"owner\":{\"idp\":\"https://localhost:9200\",\"opaqueId\":\"user1\"},\"creator\":{\"idp\":\"https://localhost:9200\",\"opaqueId\":\"user1\"},\"ctime\":{\"seconds\":\"1597323302\",\"nanos\":826749412},\"mtime\":{\"seconds\":\"1597323302\",\"nanos\":826749412}}"
  },
  "vnBcXbCwToOkXrQ": {
    "password": "",
    "share": "{\"id\":{\"opaqueId\":\"vnBcXbCwToOkXrQ\"},\"token\":\"cVglSGqBYNgDYdj\",\"resourceId\":{\"storageId\":\"1284d238-aa92-42ce-bdc4-0b0000009154\",\"opaqueId\":\"d61f729b-48d4-429e-835f-b158d6357b33\"},\"permissions\":{\"permissions\":{\"createContainer\":true}},\"owner\":{\"idp\":\"https://localhost:9200\",\"opaqueId\":\"user1\"},\"creator\":{\"idp\":\"https://localhost:9200\",\"opaqueId\":\"user1\"},\"ctime\":{\"seconds\":\"1597323435\",\"nanos\":907361770},\"mtime\":{\"seconds\":\"1597323435\",\"nanos\":907361770}}"
  }
}
PVince81 commented 4 years ago

redis state after second test:

127.0.0.1:6379> get f1651932-67dc-4874-8604-7bdaa6993111
(nil)
127.0.0.1:6379> get d61f729b-48d4-429e-835f-b158d6357b33
"/var/tmp/reva/data/user1/files/simple-folder"
PVince81 commented 4 years ago

after third run, publicshares:

{
  "MJnfTEmnaDawSWh": {
    "password": "",
    "share": "{\"id\":{\"opaqueId\":\"MJnfTEmnaDawSWh\"},\"token\":\"FByPSuDOtImhAtE\",\"resourceId\":{\"storageId\":\"1284d238-aa92-42ce-bdc4-0b0000009154\",\"opaqueId\":\"f1651932-67dc-4874-8604-7bdaa6993111\"},\"permissions\":{\"permissions\":{\"createContainer\":true}},\"owner\":{\"idp\":\"https://localhost:9200\",\"opaqueId\":\"user1\"},\"creator\":{\"idp\":\"https://localhost:9200\",\"opaqueId\":\"user1\"},\"ctime\":{\"seconds\":\"1597323302\",\"nanos\":826749412},\"mtime\":{\"seconds\":\"1597323302\",\"nanos\":826749412}}"
  },
  "uZtTFqzAfCPRfOY": {
    "password": "",
    "share": "{\"id\":{\"opaqueId\":\"uZtTFqzAfCPRfOY\"},\"token\":\"BlNOVyHlanyFpCn\",\"resourceId\":{\"storageId\":\"1284d238-aa92-42ce-bdc4-0b0000009154\",\"opaqueId\":\"764b20de-2aba-46d5-a1ab-b079365ed2ed\"},\"permissions\":{\"permissions\":{\"createContainer\":true}},\"owner\":{\"idp\":\"https://localhost:9200\",\"opaqueId\":\"user1\"},\"creator\":{\"idp\":\"https://localhost:9200\",\"opaqueId\":\"user1\"},\"ctime\":{\"seconds\":\"1597323667\",\"nanos\":470334139},\"mtime\":{\"seconds\":\"1597323667\",\"nanos\":470334139}}"
  },
  "vnBcXbCwToOkXrQ": {
    "password": "",
    "share": "{\"id\":{\"opaqueId\":\"vnBcXbCwToOkXrQ\"},\"token\":\"cVglSGqBYNgDYdj\",\"resourceId\":{\"storageId\":\"1284d238-aa92-42ce-bdc4-0b0000009154\",\"opaqueId\":\"d61f729b-48d4-429e-835f-b158d6357b33\"},\"permissions\":{\"permissions\":{\"createContainer\":true}},\"owner\":{\"idp\":\"https://localhost:9200\",\"opaqueId\":\"user1\"},\"creator\":{\"idp\":\"https://localhost:9200\",\"opaqueId\":\"user1\"},\"ctime\":{\"seconds\":\"1597323435\",\"nanos\":907361770},\"mtime\":{\"seconds\":\"1597323435\",\"nanos\":907361770}}"
  }
}

redis:

% redis-cli
127.0.0.1:6379> get f1651932-67dc-4874-8604-7bdaa6993111
(nil)
127.0.0.1:6379> get 764b20de-2aba-46d5-a1ab-b079365ed2ed
"/var/tmp/reva/data/user1/files/simple-folder"
127.0.0.1:6379> get d61f729b-48d4-429e-835f-b158d6357b33
"/var/tmp/reva/data/user1/files/simple-folder"
refs commented 4 years ago

managed to reproduce it by clickytty-clacking around, these are the server logs:

2020-08-13T15:11:52+02:00 ERR error during STAT id: storage_id:"1284d238-aa92-42ce-bdc4-0b0000009154" opaque_id:"74779be8-6c09-4f63-8998-ca0e73d4efd5"  pkg=rgrpc rpc_code=CODE_NOT_FOUND service=reva traceid=dc5bd276e582028db752275335640b0b
2020-08-13T15:11:52+02:00 INF unary code=OK end="13/Aug/2020:15:11:52 +0200" from=tcp://[::1]:65123 pkg=rgrpc service=reva start="13/Aug/2020:15:11:52 +0200" time_ns=1421022 traceid=dc5bd276e582028db752275335640b0b uri=/cs3.gateway.v1beta1.GatewayAPI/Stat user-agent=grpc-go/1.26.0

The main reason is that when querying redis with an opaqueid from a stray share (stray share = share that points to a dead resource) redis entry is nil, and the STAT fails, returning an error.

PVince81 commented 4 years ago

so this means that as soon as there is one stray share, the "not found" error bubbles up and causes the "shares.go" to return an empty set

PVince81 commented 4 years ago

okay, so this matches https://github.com/cs3org/reva/issues/1087

refs commented 4 years ago

open PR: https://github.com/cs3org/reva/pull/1090

PVince81 commented 4 years ago

after running all tests, the number of failures got down to 2. now when running those 2 tests independently (with dirty env):

1) Scenario: user removes the public link of a file using webUI # tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature:599                                                                                                                                  
   ✔ Before # tests/acceptance/setup.js:28
   ✔ Before # tests/acceptance/setup.js:32
   ✔ Before # tests/acceptance/setup.js:36
   ✔ Before # tests/acceptance/setup.js:46
   ✔ Before # tests/acceptance/setup.js:68
   ✔ Before # tests/acceptance/setup.js:75
   ✔ Before # tests/acceptance/stepDefinitions/filesContext.js:18
   ✔ Before # tests/acceptance/stepDefinitions/generalContext.js:228
   ✔ Before # tests/acceptance/stepDefinitions/generalContext.js:267
   ✔ Given user "user1" has been created with default attributes # tests/acceptance/stepDefinitions/provisioningContext.js:147
   ✔ Given user "user1" has logged in using the webUI # tests/acceptance/stepDefinitions/loginContext.js:67
   ✔ And user "user1" has created a public link with following settings # tests/acceptance/stepDefinitions/sharingContext.js:567
       | path        | lorem.txt   |
       | name        | Public-link |
       | permissions | read        |
   ✔ When the user removes the public link named "Public-link" of file "lorem.txt" using the webUI # tests/acceptance/stepDefinitions/publicLinkContext.js:177
   ✖ Then user "user1" should have some public shares # tests/acceptance/stepDefinitions/publicLinkContext.js:86
       AssertionError [ERR_ASSERTION]: Shares not found
           at _callee9$ (/home/vincent/Private/Work/workspace/phoenix/tests/acceptance/stepDefinitions/publicLinkContext.js:89:12)
           at tryCatch (/home/vincent/Private/Work/workspace/phoenix/node_modules/regenerator-runtime/runtime.js:45:40)
           at Generator.invoke [as _invoke] (/home/vincent/Private/Work/workspace/phoenix/node_modules/regenerator-runtime/runtime.js:274:22)
           at Generator.prototype.(anonymous function) [as next] (/home/vincent/Private/Work/workspace/phoenix/node_modules/regenerator-runtime/runtime.js:97:21)
           at asyncGeneratorStep (/home/vincent/Private/Work/workspace/phoenix/node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
           at _next (/home/vincent/Private/Work/workspace/phoenix/node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)
           at process._tickCallback (internal/process/next_tick.js:68:7)
   ✔ After # tests/acceptance/stepDefinitions/provisioningContext.js:221
   ✔ After # tests/acceptance/stepDefinitions/generalContext.js:272
   ✔ After # tests/acceptance/stepDefinitions/generalContext.js:252
   ✔ After # tests/acceptance/setup.js:110
   ✔ After # tests/acceptance/setup.js:106
   ✔ After # tests/acceptance/setup.js:102
   ✔ After # tests/acceptance/setup.js:95
   ✔ After # tests/acceptance/setup.js:86
   ✔ After # tests/acceptance/setup.js:54

and

1) Scenario: sharing details of indirect link share in "favorites" file lists # tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature:904                                                                                                                    
   ✔ Before # tests/acceptance/setup.js:28
   ✔ Before # tests/acceptance/setup.js:32
   ✔ Before # tests/acceptance/setup.js:36
   ✔ Before # tests/acceptance/setup.js:46
   ✔ Before # tests/acceptance/setup.js:68
   ✔ Before # tests/acceptance/setup.js:75
   ✔ Before # tests/acceptance/stepDefinitions/filesContext.js:18
   ✔ Before # tests/acceptance/stepDefinitions/generalContext.js:228
   ✔ Before # tests/acceptance/stepDefinitions/generalContext.js:267
   ✔ Given user "user1" has been created with default attributes # tests/acceptance/stepDefinitions/provisioningContext.js:147
   ✔ Given user "user1" has created a public link with following settings # tests/acceptance/stepDefinitions/sharingContext.js:567
       | path | /simple-folder |
       | name | Public Link    |
   ✔ And user "user1" has logged in using the webUI # tests/acceptance/stepDefinitions/loginContext.js:67
   ✔ When the user browses to the shared-with-others page # tests/acceptance/stepDefinitions/filesContext.js:50
   ✖ Then the folder should be empty on the webUI # tests/acceptance/stepDefinitions/filesContext.js:524
       Error while running "equal" command: Failed [equal]: (1 == 0) - expected "0" but got: "1" (undefinedms)
           at _callee25$ (/home/vincent/Private/Work/workspace/phoenix/tests/acceptance/stepDefinitions/filesContext.js:526:24)
           at process._tickCallback (internal/process/next_tick.js:68:7)
   ✔ After # tests/acceptance/stepDefinitions/provisioningContext.js:221
   ✔ After # tests/acceptance/stepDefinitions/generalContext.js:272
   ✔ After # tests/acceptance/stepDefinitions/generalContext.js:252
   ✔ After # tests/acceptance/setup.js:110
   ✔ After # tests/acceptance/setup.js:106
   ✔ After # tests/acceptance/setup.js:102
   ✔ After # tests/acceptance/setup.js:95
   ✔ After # tests/acceptance/setup.js:86
   ✔ After # tests/acceptance/setup.js:54

1 scenario (1 failed)
5 steps (1 failed, 4 passed)

I'll rerun everything again with a clean env to see if they still happen.

refs commented 4 years ago

the second failure could be related to the versions...

PVince81 commented 4 years ago

first test case is a demonstration of https://github.com/owncloud/ocis-reva/issues/389, so we can adjust the test as this is fixed.

cc @individual-it

PVince81 commented 4 years ago

these were bug demonstration tests, I've now adjusted them here: https://github.com/owncloud/phoenix/pull/3948