Group rename bulk update bug whatever | LuckPerms | Page 1

left epoch Aug 4, 2021, 8:04 AM

#

@molten python wdym?

molten python Aug 4, 2021, 8:04 AM

#

like if you had the code like this instead:

#

let me get it together

left epoch Aug 4, 2021, 8:04 AM

#

Sure

molten python Aug 4, 2021, 8:05 AM

#

StorageAssistant.save(newGroup, sender, plugin);
if (args.contains("--update-parent-lists")) {
  // the group is now renamed, proceed to update its representing inheritance nodes
  BulkUpdate operation = BulkUpdateBuilder.create()
    .trackStatistics(false)
    .dataType(me.lucko.luckperms.common.bulkupdate.DataType.ALL)
    .action(UpdateAction.of(QueryField.PERMISSION, Inheritance.key(newGroupName)))
    .query(Query.of(QueryField.PERMISSION, Constraint.of(StandardComparison.EQUAL, Inheritance.key(target.getName()))))
    .build();
  plugin.getStorage().applyBulkUpdate(operation).whenCompleteAsync((v, ex) -> {
      plugin.getSyncTaskBuffer().requestDirectly();
      if (ex != null) ex.printStackTrace();
  }, plugin.getBootstrap().getScheduler().async());
}

would that be problematic?

#

because #save is being called with newGroup as a parameter, so wouldnt that just be saving the newly added nodes from the recreation? Why does it have to be called after the operation, when the operation doesnt affect newGroup

left epoch Aug 4, 2021, 8:08 AM

#

Well again I don't remember exactly but I probably decided it should be called last due to uncertainty of whether it runs on another thread or not.
It definitely should be saved before the bulk update, but also make sure the bulk update runs after saving is completed

molten python Aug 4, 2021, 8:09 AM

#

#save calls GroupManager#saveGroup(Group) which returns a CompletableFuture

left epoch Aug 4, 2021, 8:09 AM

#

Group rename bulk update bug whatever

left epoch Aug 4, 2021, 8:10 AM

#

molten python #save calls GroupManager#saveGroup(Group) which returns a CompletableFuture

Is the return value discarded or does #save return it too?

molten python Aug 4, 2021, 8:10 AM

#

it's discarded

#

they call #get on it, and if that throws an exception it returns

left epoch Aug 4, 2021, 8:11 AM

#

Oh so it gets

#

Fun, okay this is easy then

molten python Aug 4, 2021, 8:11 AM

#

incase that helps

#

That suspends the current thread until saving is complete right?

left epoch Aug 4, 2021, 8:13 AM

#

molten python ```java StorageAssistant.save(newGroup, sender, plugin); if (args.contains("--up...

Yeah, so that should work fine then

molten python Aug 4, 2021, 8:14 AM

#

I dont have a cloned LP, and I'll probably fuck up a PR anyhow lmao

#

though that code is copy pasted and just modified so u should be able to toss it in there when you're off phone

left epoch Aug 4, 2021, 8:15 AM

#

Lmao okay and yeah

#

Never thought something as catastrophic as removing all of the group's permissions would ever happen . . .

molten python Aug 4, 2021, 8:16 AM

#

also, why does #save(Group) invalidate the cache for every group&user, instead of just the one being saved? 🤔

left epoch Aug 4, 2021, 8:17 AM

#

Inheritance, permissions are precalculated and cached

#

So it re does them

#

I think

#

Lmao

molten python Aug 4, 2021, 8:19 AM

#

right but say you have

groupOne.setNodes(DataType.NORMAL, target.normalData().asList(), false);
groupTwo.setNodes(DataType.NORMAL, target.normalData().asList(), false);

StorageAssistant.save(groupOne, sender, plugin);
StorageAssistant.save(groupTwo, sender, plugin); # this

wont the saving of groupTwo be ruined because the nodes are only in cache, which save deletes?

left epoch Aug 4, 2021, 8:23 AM

#

Yes, that's why you save them as you modify them :P

#

Or

#

Wait

#

I don't think so, since it isn't reloading them from storage, just recalculating

#

It's not a (storage) sync task request, just invalidate cached calculations

molten python Aug 4, 2021, 8:33 AM

#

🤔 I think I'm missing something vital

#

what counts as cache, are these node changes not only stored in cache, and by deleting the cache wont they disappear

left epoch Aug 4, 2021, 8:35 AM

#

No

#

What's saved in storage is basically the set of nodes a holder (user/group) holds

#

When the holder is loaded into memory, it loads the nodes "raw" into the node map/set

#

The cached values instead takes that node map and perform the necessary calculations based on active contexts (server, world, gamemode etc)

#

The cached data does not necessarily represent the loaded data from storage

#

The idea of it being cached is that it doesn't mutate, since it'd need to perform the tree traversal, wildcards, shorthand, regex, contexts, and whatnot for every perm check

#

So when you invalidate caches, you force recalculation, and not reload the data from storage onto memory

molten python Aug 4, 2021, 8:44 AM

#

Ah okay I get it now

#

thank you Emily!

left epoch Aug 4, 2021, 8:45 AM

#

https://tenor.com/view/peepo-shy-pepe-shy-peepo-blush-peepo-gif-19500792

Tenor

left epoch Aug 6, 2021, 3:36 AM

#

okay so I found what I was talking about, and why I was so uncertain etc

left epoch Aug 6, 2021, 3:37 AM

#

molten python incase that helps

see this? more specifically this bit at the bottom

Optional<InternalMessagingService> messagingService = plugin.getMessagingService();
if (messagingService.isPresent() && plugin.getConfiguration().get(ConfigKeys.AUTO_PUSH_UPDATES)) {
    messagingService.get().getUpdateBuffer().request();
}

#

that request() returns a CompletableFuture and it's getting discarded 🥲 I mean since it's internal and not API I can just adapt it so it's returned but the idea behind the storageassistant is kinda to just "throw this in there and let it do its thing"

molten python Aug 6, 2021, 3:38 AM

#

left epoch see this? more specifically this bit at the bottom ```java Optional<InternalMess...

doesnt that just tell other servers to read storage?

#

I dont see why that part in particular would prevent it from being saved

left epoch Aug 6, 2021, 3:39 AM

#

molten python doesnt that just tell other servers to read storage?

yeah but you know, threading and stuff, I wouldn't want the bulk operation to run as the messaging request is happening

molten python Aug 6, 2021, 3:39 AM

#

oh

left epoch Aug 6, 2021, 3:39 AM

#

this is pain

molten python Aug 6, 2021, 3:40 AM

#

well making it return something would be backwards compatible no?

left epoch Aug 6, 2021, 3:40 AM

#

yeah totally

molten python Aug 6, 2021, 3:40 AM

#

I see, then yeah you could make it return and then tag on a thenrun or whatever it is lmao

left epoch Aug 6, 2021, 3:41 AM

#

idk every time I change something in LP's code I try to make as few changes as I possibly can xD

#

in fear of breaking something

#

WHICH I ALERWADY DIUD

#

FDNKILOFDSAFSA

molten python Aug 6, 2021, 3:41 AM

#

lmaooo

#

~~just adding a new flag couldve ruined an entire permissions setup~~

left epoch Aug 6, 2021, 3:41 AM

#

🥲

#

this dude's lost ALL their group perms because of this xD

#

good thing they had an old editor session open lmao

molten python Aug 6, 2021, 3:42 AM

#

thats good

left epoch Aug 6, 2021, 3:43 AM

#

by far non ideal

#

lol

#

woah

#

Dyno logs things happening in this thread ... even through it's not here

#

fancy

#

!whylp

#

🥲

molten python Aug 6, 2021, 3:46 AM

#

🤔 thats odd

#

@dawn sinew

#

well I've invited it

left epoch Aug 6, 2021, 4:11 AM

#

ngl i'm scared to break something else lmao

dawn sinewBOT Aug 6, 2021, 4:12 AM

#

dynoError The Fun module is disabled in this server. Enable it to use this command.

left epoch Aug 6, 2021, 4:55 AM

#

you know

#

I should have tested this crap before making the PR :^)

#

and by "this" I mean the fix https://github.com/lucko/LuckPerms/pull/3129

#Group rename bulk update bug whatever