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

Fix #2136: update cache and group list when copying pads #2174

Merged
merged 2 commits into from
Jul 8, 2014

Conversation

marcelklehr
Copy link
Contributor

Should fix #2136 -- please test, though!

(Sorry for the line noise)

@JohnMcLear
Copy link
Member

Awesome thanks @marcelklehr will try to test this week.

@eholzinger
Copy link

I've finally played around with this and have run across some issues. Could well be it's something I'm doing wrong, but here's what's happening on my server (ubuntu 10.04, db is mongo), followed by log data:

Whether attempting to copy a pad from one group to another or within a single group, the operation fails and kills the etherpad daemon. The groups all exist, so the error "destinationID false" isn't because of that.

Logs:
[2014-07-07 14:20:48.779] [INFO] API - REQUEST, v1.2.9:copyPad, {"sourceID":"g.79eMPApQaMz5OHxt$newone","destinationID":"g.lAnpebt6twMXEv8A$anewone","apikey":"bunchofrandomchars"}
[2014-07-07 14:20:48.787] [INFO] console - destinationID 'g.lAnpebt6twMXEv8A$anewone' false
[2014-07-07 14:20:48.794] [ERROR] console - ReferenceError: padID is not defined
at async.series.callback.padID (/var/www/etherpad-lite/src/node/db/Pad.js:555:66)
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:486:21
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:185:13
at iterate (/var/www/etherpad-lite/src/node_modules/async/lib/async.js:108:13)
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:119:25
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:187:17
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:491:34
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:190:13
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:94:25
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:187:17
[2014-07-07 14:20:48.794] [INFO] console - graceful shutdown...
[2014-07-07 14:20:48.868] [INFO] ueberDB - Flushed 2 values
[2014-07-07 14:20:48.981] [INFO] console - db sucessfully closed.

Same result with movePad.
And here it is copying within a group, with a new name:

[2014-07-07 14:24:02.369] [INFO] API - REQUEST, v1.2.9:copyPad, {"sourceID":"g.79eMPApQaMz5OHxt$newone","destinationID":"g.79eMPApQaMz5OHxt$newonecopy","apikey":"randomcharshere"}
[2014-07-07 14:24:02.379] [INFO] console - destinationID 'g.79eMPApQaMz5OHxt$newonecopy' false
[2014-07-07 14:24:02.385] [ERROR] console - ReferenceError: padID is not defined
at async.series.callback.padID (/var/www/etherpad-lite/src/node/db/Pad.js:555:66)
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:486:21
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:185:13
at iterate (/var/www/etherpad-lite/src/node_modules/async/lib/async.js:108:13)
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:119:25
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:187:17
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:491:34
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:190:13
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:94:25
at /var/www/etherpad-lite/src/node_modules/async/lib/async.js:187:17
[2014-07-07 14:24:02.385] [INFO] console - graceful shutdown...
[2014-07-07 14:24:02.448] [INFO] ueberDB - Flushed 2 values
[2014-07-07 14:24:02.561] [INFO] console - db sucessfully closed.

Groups all exist:
[2014-07-07 14:26:19.210] [INFO] API - REQUEST, v1.2.9:listAllGroups, {"apikey":"yada-yada-yada"}
[2014-07-07 14:26:19.215] [INFO] API - RESPONSE, listAllGroups, {"code":0,"message":"ok","data":{"groupIDs":["g.rerY4LxVbm1dHkQv","g.dqfIuiwjoGrC3Kd9","g.cUOXDdjjcAzwDYy3","g.KIfEkNcmJuriOXX9","g.jKwXnKzzZhDWIMTP","g.lTrhdGftCEf8ayAX","g.L9TTde9yL8OXSKrW","g.6uyrLW19zC4m9MyC","g.p4y9okQH08vLpMUU","g.EEOuXf5zmmYzYTAj","g.kWg8MpehWyiQlZs1","g.1KZlwarofi0CImjr","g.HZedyiQiEhbzB6yq","g.duudsNxEUA5iOBi8","g.FwRRYZ9NunVnjFJL","g.lAnpebt6twMXEv8A","g.4O3fS2lyd1rMcXZf","g.Pk1UNBSWddgGXRl9","g.m8E6nymfRKC3dnYO","g.qQxAZiEX3I6crYh1","g.1fyJ174bWD7VuTV1","g.v8N7CM3QnacjTD5k","g.bF43prYsGqQyvFCi","g.3hmYcrLzU7sjte4S","g.xGUP1mwUOg6X65pQ","g.vcC8szjNVAQUFyo0","g.iu3mDIFU1QlWQpLj","g.JLNz7F48X9WTca3x","g.dORj5rS1og0sY94s","g.UzVf9SAHB2p1pjiA","g.vAoxWxao9yrNfC2R","g.NhrOUdzQcaWXTScw","g.A6MAMk7oH3a8SRjE","g.MkpswtnOjk2f8EEk","g.mby44YsIJXGQvXfa","g.koT4DWYjAIvKcG8Z","g.uqZp5ej5J1NuD26C","g.sBdqDlSEBhSHuCEF","g.Yah7wk6DYTGpt5qV","g.tH4gttm3feCP4gKZ","g.YPlKfmRoLdPokqRa","g.aeEmBmmLqlOuukRt","g.yCQJqUJXkNtx7pod","g.5acWG5bZebfNHPbC","g.RrtiQ8qLAihYjlZF","g.OuKYSqdGmfQoeOhK","g.1lJrJuDkm2lAzbJC","g.dApM9NN8jKXzzcnb","g.xiKU1Pj0px2AKJJD","g.ZKtby7wShtaIj00E","g.vplBdZoUktsqFfWe","g.rQMpmez2uVMR2oDZ","g.W8HH3TPjw6wRCsFt","g.23bb56liZEh8Myq1","g.TkgAfB52fKT1AFKg","g.79eMPApQaMz5OHxt"]}}

Also, it seems that the api call listAllPads is now broken -- returning nothing when the listPads(groupID) call shows all the pads in the group identified by groupID.

Sorry it took me so long to get to this...

@@ -547,6 +550,13 @@ Pad.prototype.copy = function copy(destinationID, force, callback) {
// parallel
], callback);
},
function(callback) {
// Group pad? Add it to the group's list
if(destGroupID) db.setSub("group:" + destGroupID, ["pads", padID], 1);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess this should read destinationID

@marcelklehr
Copy link
Contributor Author

Thanks for testing. Should be fixed now.

Are you sure that the commit(s) in this pr break listAllPads?

@eholzinger
Copy link

No, I'm not 100% sure. But it's working on a production server running 1.4.0-9-g20c32de...

Meanwhile, i'll give the copy/move another go. Thanks for working on this, btw, especially so quickly. (am a little chagrined that you jumped right on it after I let it slide for a few weeks!)

@eholzinger
Copy link

Works like a champ! Both for copying between different groups and within groups as well as moving from one group to another, renaming or not. Thanks much!

marcelklehr added a commit that referenced this pull request Jul 8, 2014
Fix #2136: update cache and group list when copying pads
@marcelklehr marcelklehr merged commit 9d1eca4 into develop Jul 8, 2014
@marcelklehr
Copy link
Contributor Author

alrighty, Glad it works now :)

@JohnMcLear
Copy link
Member

Thanks for the fix @marcelklehr you are the man! :)

@Gared Gared deleted the fix/copy-pad-update-lists branch December 26, 2014 23:09
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

Successfully merging this pull request may close these issues.

copyPad changes not reflected in subsequent calls to list pads
3 participants