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

send share notification instead of erroring on duplicate share #25533

Merged
merged 1 commit into from
Mar 9, 2021

Conversation

icewind1991
Copy link
Member

when creating a share that already exists, instead of erroring, resend the notifications

Signed-off-by: Robin Appelman robin@icewind.nl

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Feb 8, 2021
@icewind1991 icewind1991 added this to the Nextcloud 22 milestone Feb 8, 2021
@nickvergessen
Copy link
Member

Need to test and confirm this doesn't break Talks reposting behaviour

@icewind1991 icewind1991 force-pushed the resend-share-notifications-on-recreate branch from b673658 to 65b3539 Compare February 9, 2021 14:56
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Seems to work

@faily-bot
Copy link

faily-bot bot commented Feb 9, 2021

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 2076: failure

checkers

integration-ratelimiting

  • failure block could not be found - most likely this run got canceled
Show full log
+ bash tests/drone-run-integration-tests.sh || exit 0
=========================
= List of changed files =
=========================
lib/composer/composer/autoload_classmap.php
lib/composer/composer/autoload_static.php
lib/private/Share20/Manager.php
lib/public/Share/Exceptions/AlreadySharedException.php
=========================
PHP files are modified
+ ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
Nextcloud was successfully installed
+ ./occ config:system:set redis host --value=cache
System config value redis => host set to string cache
+ ./occ config:system:set redis port --value=6379 --type=integer
System config value redis => port set to integer 6379
+ ./occ config:system:set redis timeout --value=0 --type=integer
System config value redis => timeout set to integer 0
+ ./occ config:system:set --type string --value "\OC\Memcache\Redis" memcache.local
System config value memcache.local set to string \OC\Memcache\Redis
+ ./occ config:system:set --type string --value "\OC\Memcache\Redis" memcache.distributed
PHP Warning:  Redis::connect(): php_network_getaddresses: getaddrinfo failed: Name or service not known in /drone/src/lib/private/RedisFactory.php on line 92
An unhandled exception has been thrown:
RedisException: Redis server went away in /drone/src/lib/private/Memcache/Redis.php:55
Stack trace:
#0 /drone/src/lib/private/Memcache/Redis.php(55): Redis->get('4ce41d68edfec2e...')
#1 /drone/src/lib/private/App/InfoParser.php(60): OC\Memcache\Redis->get('/drone/src/apps...')
#2 /drone/src/lib/private/App/AppManager.php(511): OC\App\InfoParser->parse('/drone/src/apps...')
#3 /drone/src/lib/private/legacy/OC_App.php(589): OC\App\AppManager->getAppInfo('files', false, NULL)
#4 /drone/src/lib/private/AppFramework/App.php(70): OC_App::getAppInfo('files')
#5 /drone/src/lib/private/legacy/OC_App.php(270): OC\AppFramework\App::buildAppNamespace('files')
#6 /drone/src/lib/private/AppFramework/Bootstrap/Coordinator.php(108): OC_App::registerAutoloading('files', '/drone/src/apps...')
#7 /drone/src/lib/private/AppFramework/Bootstrap/Coordinator.php(82): OC\AppFramework\Bootstrap\Coordinator->registerApps(Array)
#8 /drone/src/lib/base.php(634): OC\AppFramework\Bootstrap\Coordinator->runInitialRegistration()
#9 /drone/src/lib/base.php(1076): OC::init()
#10 /drone/src/console.php(49): require_once('/drone/src/lib/...')
#11 /drone/src/occ(11): require_once('/drone/src/cons...')

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Sure

@rullzer
Copy link
Member

rullzer commented Feb 11, 2021

Ah you need the since annotation I guess

@rullzer rullzer force-pushed the resend-share-notifications-on-recreate branch 2 times, most recently from ea84247 to 3da33ce Compare March 9, 2021 18:20
when creating a share that already exists, instead of erroring, resend the notifications

Signed-off-by: Robin Appelman <robin@icewind.nl>
@rullzer rullzer merged commit fad86af into master Mar 9, 2021
@rullzer rullzer deleted the resend-share-notifications-on-recreate branch March 9, 2021 18:44
@MorrisJobke
Copy link
Member

/backport to stable20

@MorrisJobke
Copy link
Member

/backport to stable21

@skjnldsv
Copy link
Member

skjnldsv commented Apr 12, 2021

@icewind1991 @nickvergessen @rullzer
Now we can add the same share an unlimited amount of times.
Having the api NOT throw on conflict seems like a bad idea to me. Please fix this for 22!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants