LuckPerms

LuckPerms

917k Downloads

Unable to modify existing meta nodes without clearing first.

bloodmc opened this issue ยท 1 comments

commented

LP 5.0.63
Spigot 1.15.2

Consider the data

  "meta": [
    {
      "key": "griefdefender.accrued-blocks",
      "value": "2500",
      "context": {
        "server": "global"
      }
    },
    {
      "key": "griefdefender.bonus-blocks",
      "value": "0",
      "context": {
        "server": "global"
      }
    }
  ]

I would expect the following code to modify an existing meta node on a permission holder

MetaNode.builder().key(key).value(value).context(set).build();
permissionHolder.data().add(node);

If I use the key griefdefender.accrued-blocks with a new value of 5000 and context server:global it will create the following

  "meta": [
    {
      "key": "griefdefender.accrued-blocks",
      "value": "2500",
      "context": {
        "server": "global"
      }
    },
    {
      "key": "griefdefender.accrued-blocks",
      "value": "5000",
      "context": {
        "server": "global"
      }
    },
    {
      "key": "griefdefender.bonus-blocks",
      "value": "0",
      "context": {
        "server": "global"
      }
    }
  ]

As you can see from above, 2 meta entries exist with identical key and context. The only workaround for the above is to first clear any existing meta using key/context then set a new one. This solution is OK for file storage like JSON but with DB storage like mysql it will auto-increment the primary key field as a new record is being added to the table. Thoughts?

commented

From an API design perspective, I prefer the removal of the existing (replaced) node to be explicit.
i.e.

user.data().clear(NodeType.META.predicate(node -> node.getMetaKey().equals("griefdefender.accrued-blocks")));
user.data().add(MetaNode.builder("griefdefender.accrued-blocks", "5000").build());

However, the API design doesn't affect the way the storage is implemented - since both systems are abstracted from each other. Even if an API method were added which allowed meta values to be "updated" in one method call, the SQL storage implementation would likely still delete+insert as opposed to updating - atm it just performs a simple diff using .equals().