synac-chat / synac-legacy

Some IRC-like chat application that might become good one day
11 stars 1 forks source link

Should there be a bulk delete endpoint and should it be separate from the delete one? #12

Closed jD91mZM2 closed 6 years ago

Mnpn commented 6 years ago

Add it, but separate.

jD91mZM2 commented 6 years ago

Care to argue over why it should be separate? Would it require special permissions? (off-topic) Would it have a limit?

ttofis commented 6 years ago

More packet types.... Ugh... Just allow anyone with the permission of delete to send a package the delete package BUT instead of a message id it can accept and an array of strings with the from-to message ids.

jD91mZM2 commented 6 years ago

array of usizes* :stuck_out_tongue:

but yes, that is what I thought too. @Mnpn03 thoughts?

ttofis commented 6 years ago

Sorry did you mean uintptr :sweat_smile:

jD91mZM2 commented 6 years ago

Oh we're going to do a language war? Go has one of the largest complains on YourLanguageSucks. Hah, take that! :stuck_out_tongue:

ttofis commented 6 years ago

Oh well... Lemme continue my rapid development of my golang lib with instantaneous line by line recompiling to achieve exactly what I want while you are waiting on compiling changes of: OOPS you mispelled your function!

Mnpn commented 6 years ago

Vouch

jD91mZM2 commented 6 years ago

@ELChris414 Sorry, I didn't quite understand. Am I mispelling functions? Do tell me where, if so.

ttofis commented 6 years ago

Nahh man, I just made a joke to how slow rust compiles stuff

jD91mZM2 commented 6 years ago

Should there be a limit in how many messages can be deleted? Should deleting multiple messages be rate limited as "expensive" so you can't repeat it as often?

ttofis commented 6 years ago

Well, limits... In a program where you do the server hosting, you should be the one to choose if it causes that much of a performance downtime.. So, I believe it shouldn't have any limit.

jD91mZM2 commented 6 years ago

In other words you are saying you want it to have a limit in the config. But we need a defaut value.

ttofis commented 6 years ago

No, in other words I am saying it shouldn't have a limit and the owner should know to what people does he give the delete perm

jD91mZM2 commented 6 years ago

But people can delete their own comments @ELChris414. Either way a perm isn't made to say "I trust this person", it's a value of how much you trust that person. You may trust somebody to manage messages without trusting them to be able to delete the whole message history of the server in less than a day, and freeze the whole thread while doing so.

ttofis commented 6 years ago

Hmm.. So, I understand what are you trying to say. Then, a limit of 100 messages deleted every 30 minutes (work around the correct timing for minutes or whatever time schedule you like to work with) should do it I think.

jD91mZM2 commented 6 years ago

Aka I'll count this as an "expensive" function. "expensive" functions are by default rate limited to like 2 times an hour or so, I don't remember

ttofis commented 6 years ago

That's sufficient I say! But the bulk delete function should have a limit to the amount itself I guess. 100 sounds good.

jD91mZM2 commented 6 years ago

I'd say 64 or 128 because it's a power of 2 :>

ttofis commented 6 years ago

Do it! (close issue too)

jD91mZM2 commented 6 years ago

Now is the challenge to avoid writing repetetive code D:

jD91mZM2 commented 6 years ago

Unexpected pitfall #​1: MessageDelete currently gets the channel from the ID (because IDs are unique per channel). It uses the channel to look at the channel overrides. But if we change it to use multiple values, we could get multiple channels. We obviously don't want to query the channel for every message.

Solutions:

Please recommend a solution or invent your own @ELChris414.

ttofis commented 6 years ago

So, quick question... Why do you need the message's channel to delete the message? Is it because of the way it is stored in the database? So, even if we go for the second solution you will have to disallow I guess the deletion of messages that do not exist in that channel. And by disallow, I mean just skip. But that raises another question: How does once say which messages should be deleted? I mean, is it a from to? That wouldn't make sense. The way it could work is the client knowing which messages belong to which channels and giving them 1by1. If so, I say the second solution is sufficient but also make it skip messages that don't belong to that channel so you don't get a crash :P

jD91mZM2 commented 6 years ago

I need the channel in order to get the channel overrides to check permissions. And by disallow, you mean throw a hostile error at the client that the channel isn't equal to the channel of the message.

ttofis commented 6 years ago

Ok, but you will sadly have to check each and every message if it belons in that channel, because somebody could theoretically give as a channel a channel where he has the permission but delete messages from a channel that he doesn't.

jD91mZM2 commented 6 years ago

I have to check each message either way, to make sure it exists. SQL makes this easy enough.

ttofis commented 6 years ago

Cool, so I say the second solution plus the 1 or 2 warnings I gave you!

tbodt commented 6 years ago

I like the first one, if the client doesn't know what channel it's deleting from that needs to be fixed

jD91mZM2 commented 6 years ago

I mean, the client knows which channel it's in, but a client can give a message ID from another channel. So to know what messages are in what channels, I'd need a message -> channel HashMap, which is very RAM inefficient.

@tbodt

tbodt commented 6 years ago

no, put the channel in the message data structure

jD91mZM2 commented 6 years ago

It is, but passed messages are not saved in structure, they're saved as log entires