-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Support wildcard classname in cloud triggers #7361
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
Comments
Thanks for suggesting. Please use the issue template provided and describe the feature in more detail. The PR already makes the assumptions that the "trigger will be called for every class unless class has its own trigger". This and possibly other aspects should be discussed in the issue. |
@mtrezza I have edited the issue. |
I think the obvious alternative would be to define a function that contains the recurring logic. Can you explain the assumption that these generic Cloud Function definitions override a class specific definition? |
You mean something like this?
Or even like this:
But user is still have to define trigger for every class.
For example all classes has a user pointer. And Developer may want to assign the user that created the parse object. Developer can do this with this code:
This is a simple code but if developer has many classes, defining the triggers manually would take too long. If developer has 100 classes, he/she has to write same code 100 times. But I admit that 99% of projects doesn't go beyond 15-25 classes. And generally classes has seperate logic. But this feature would make coding easier for those who need it. Its rare. But since someone opened a thread on community forum, that means someone need it. |
Yes, I wanted to mention it as an alternative, so we understand what the status quo is. The issue says that no alternatives exist.
I agree, it seems to be a useful addition to Parse Server.
I would like to understand how you came to the assumption that these generic Cloud Function definitions should override a class specific definition. If I understand you correctly this means:
|
User still have to define triggers manually so not an exactly alternative.
This is because developer may want to seperate some class logic from the others. Just because you used wildcard triggers doesn't mean you cant use class specific triggers. If developer has a common logic for 50 class but different logic for a few class, they can use generic triggers for that classes. Like this:
We are making the job easy while not limiting the developers options.
I think you meant class specific trigger override generic one. PR looks first class specific trigger. If there is no class specific trigger, then it uses wildcard trigger.
Yep. That's correct. |
Alternative here means how it can be done currently, without this enhancement. So we know where we are coming from. Could you add this to the issue description?
We are actually doing the opposite, because we implement a fixed rule in case of multiple definitions for the same class. You can make your feature more useful to more developers if you give some freedom to cover more use cases. The question is: if we have multiple definitions (specific and generic) for a class, how should they be handled?
The above is roughly in order of complexity, meaning a) will be less work in the PR than d), but inversely, d) will be more useful to developers than a). When choosing a concept we have to look at forward compatibility, meaning if we go for a) or b) now, we may have more difficulty going to c) or d) later, because it may be a breaking change. |
Its still can't be done. Because we still have to write all class name manually which is what we are trying to escape.
I think option A would be best. Option B is in least favorable in my opinion. Because It locks developers hand. Its either black or white. No between. And about option C and D, I would choose C. I would run class specific trigger first then execute generic trigger. But running two triggers for same class doesn't make sense to me. I mean I see your point but why use generic trigger if you already defined class specific trigger?
Why would we wanna switch to the c or d in future? Can you give me an example Edit: I miss read. You can Ignore the below.
|
You could just iterate through the schema and get all class names that match a regex, that's the same.
Your user example, if you have additional per-class custom code.
Option a) and b) are equally restrictive. You may prefer a) because it works for your case, other may prefer b) for their case. The same with the order in c). The only practical option is d) in my opinion. Any idea how that can be made configurable? |
Yes. You are right.
I disagree. You can always put generic logic to class specific trigger. But you can't put class specific logic to generic trigger. If you do, it's not generic logic anymore.
I would choose A. My logic is start from the small cluster and move to bigger ones. But If we really need to run both triggers I would let developer decide that.
We already have validators. We can use that.
We can use that. But its unnecessarily complex. Current solution works for old versions. And will work for newer versions. We can put documentation that" class specific trigger will override generic trigger " or something like that. |
Not sure I follow. My point is that you want the following rule:
But someone else may want this rule:
It may be a less common use case, but there may be scenarios in which this applies. For example if someone has a deployment in which Parse Functions load conditionally. I think we need such a decision path logic anyway, because you can have multiple generic triggers that overlap.
Which one should be executed? In my opinion all, the question is just in which order. The developer just has to understand that the class name allows regex, that would be the only change in the docs. |
If you don't want the specific trigger to run, why would you even define it in first place.
I didn't understand this. In this code,
This only executed for class Test
Right now which one parse executes? |
Currently, if you have multiple CFs, This gets more complex with regex CF, because we need to define now what should happen. In my opinion all CFs that match the class name regex should be executed. That gives the developer most versatility. It would be a straight forward PR and easy to comprehend: execute all triggers that regex match the class name. In addition, the context object is accessible from all triggers, so the developer can leverage that for more complex scenarios. If you want a specific CF to "override" the generic CF, you would simply change the regex of the generic CF to exclude a specific class name, so that only the specific CF is executed. That would be the most intuitive, straight forward behavior. Any compulsory rule (like ignore generic if there is a specific) is arbitrary and increases complexity and restrictions down the road. |
Should we create a poll on community forum? What do you say? We would see what people want. İf they want to run both triggers we run both triggers. İf they want one to override the other, we go that way. @mtrezza |
Not sure why you suggest polling? This is not a question of apple or carrot, but rather of apple or grocery store (grocery store having apples, carrots and much more). I want to make sure you understand the regex approach: The approach to implement a regex syntax and execute all matching functions covers many use cases, including your specific use case. You can set it up to not execute a specific CF. The approach you are suggesting may cover your specific use case, but it is restrictive and not versatile. That means it will be usable in less use cases. It is also problematic on other fronts:
For example, what you can do with regex:
Let me know where you see an issue with the regex approach, or why you think a restrictive approach is better. |
I think our issue here is not how we implement wildcard triggers but what will we do if multiple triggers are set for same class.
I liked the regex approach. But problem relays on what if one class has multiple triggers. Currently if multiple trigger is set to one class last one overrides other. I think we should keep this functionality. And since we cant do this on regex approach, we should run smallest cluster. For example if user defined 2 triggers and we have UserProfile class. 1- beforeSave("U*" - trigger for all classes that name starts with letter U. I think we should run only 2nd trigger. Because ıt invokes for less classes. And If user add 3rd trigger like beforeSave("UserProfiles" we should bypass 1st and 2nd. And we should run 3rd one.
To see what people really want.
But if market demand is for only apples, you lose money on others. And If we bring a feature that nobody wants, time and effort will be wasted. |
It seems you are misunderstanding the regex approach and ignoring the implications of your approach.
You keep saying "we should", but what is your argument for that? Maybe it's easier if we talk about a specific example. Can you give one example that you want to do with your approach but cannot be done with the regex approach? |
One question. Is it possible to run multiple triggers for same class with regex? Multiple |
Can you give an example, I want to make sure we talk about the same. |
My way is running only one trigger for classes. Multiple triggers can be confusing and developer can lose the track easily. And even we run multiple trigger, how will we determine the order? And ıf I understand regex way, we cant really know how many triggers will user define. My example is this:
We should run only one trigger and that trigger must represent smallest cluster. Now If you can give an example that would be great. Maybe we have a communication problem. |
Yes, in your example, all 3 triggers will run.
You can define that with regex and run only 1 trigger. What exactly is your concern if the behavior you want (and much more) can be achieved with regex? If you prefer we can take this off to Slack for a quick chat. I really think this is just a miscommunication. |
No I can't because like you said I can define class specific trigger by directly entering class name. But If I already define class specific trigger why would I want to seperate my code? PR's aim was getting rid of the defining triggers seperatly.
Sure I've downloaded it. How can I reach you? |
Your PR may do that, but it is tailored too much to your use case. Parse Server philosophy is choosing universality over restriction to make it applicable for as many use cases as possible. Also we have to think beyond this PR, regex is the logical next step after your PR, and we will run into compatibility issues if we go with that PR now.
You can join the parse community slack group. I'll be reachable there in about two hours. |
Ok. That's understandable. Do you have any idea how can we do regex implementation? This is current code in parse repo:
and this is the get unctions:
It seems like if we wanna support regex, we need to retrieve every trigger before countinue. And check them manually. How can we solve this? Or does it need to be solved? Doing a few extra steps doesnt exaust CPU.
Two hours from now is midnight here so I may not reply but I can reply tomorrow. My username is @uzaysan |
Basically there are two regex approaches:
We can go with a) for simplicity and it can be optimized in the future if someone wants to do that. The question you asked is still valid: in which order should the CFs run if multiple CFs regex match a class? Do you have any suggestion? |
Order actually doesn't matter anymore because parse will execute them all. If we were eliminate some of the triggers(like my suggestion) order was important. But since we will run them all, we can run them all without a specific order.
Option A looks better than B. |
Order can matter depending on what the trigger does. We may well say that the execution order is undefined for now. That simplifies the PR, leaving the door open for future change without breaking anything. |
Currently we have this triggers:
Since this triggers will only run for one class or for one specific thing, we should leave them.
Regex will only apply to these triggers
Is this sounds good? Edit: Looks like all triggers (except file trigger) use same |
Yes, I think these make sense, they are the only class related triggers it seems. |
So the PR needs to add is a regex test condition and iterate over all definition names. We may need to think of how to store the class definitions, currently definitions are only chars and numbers I think, but a regex definition could be a string or a |
I was thinking something like this. Syntax will be "Cla*". and this trigger will run for all classes that starts with Cla. My example code:
This function will look for this triggers:
And in İnstead of this :
We do this:
This function assume that syntax will be my suggestion. |
This is not regex syntax. Can you come up with a regex approach? |
Can you give me an example? And also whats wrong with this approach? It does what we want. Doesn't it? |
You would have to get familiar with what regex is and how it works to design a solution for this issue.
No it doesn't. How would you run a trigger for all class names that start with |
I know what regex is. But parse gets trigger with
Can you give an example that regex can do but this approach cant? |
How would you run a trigger for all class names that start with |
Do you have any idea how can we implement regex to this: |
It seems we are going in circles so let's settle some points:
As I suggested previously, the next step would be to look at how triggers are stored and what needs to be changed. |
So you are saying that
And internally we can retrieve all beforeSave triggers. And chech if regex matches like this:
Then we run all triggers inside of the array. Are you talking about something like this? |
Yep! That's what I was thinking of, although I don't fully understand the code above. I just assume that we need to account for some cases. For example,
I think b) makes more sense. To clarify, there are:
Currently, we have only (a) and we are now adding (b), which is not the same as (a). We need to consider the difference between (a) and (b), because (a) is actually |
If we want to support regex, that means we use regex string as object key. So our object will look like this
So first we have to extract regex strings from object. And if classname is now equal, we can assume its regex altough it may not. But since it will not match other strings we can ignore that. And our final code will look like this
We will return array of trigger for all triggers: And in
|
Conceptually yes, but a regex cannot be used as JS object key, so the code above is unlikely to work as it is. Do you want to start a PR and see how to get it working? |
Sure. Let me see what I can do. |
Somehow I missed this thread going on but I think it was triggered by my request on the community forum. My summary of your discussion and points related to it:
So in this case: and Regarding hooks-vs-behaviors-and-traits - the idea of specifying the behavior of class type based on the hooks called this way may be considered counter-intuitive by those used to the ORM world and data modelling. IMHO, it would have been much better - or would become much better - if the behaviors/traits were class-based: I realize that's a breaking change, but it would have been/would be a better design pattern IMHO. |
@tremendus thanks for your comment. I want to iterate 3 points here that we have already discussed at length in this thread:
Maybe you can give a more detailed example use case, so we can look into that. If what you are suggesting is something like a To your specific points:
If you mean "on instantiating" the user, this can already be achieved with subclassing. If you mean "on saving" the user that would be a collection based hook, i.e. a beforeSave User, as it already exists.
Could you elaborate on that? In Parse Server, a "class" is essentially a fusion of a programmatic "object class" and a DB "storage collection". If you suggest that it should be possible to do |
Sorry for getting late in this thread ... I would suggest to avoid as much as possible using strings definitions for classes names. Simply for a type-safety perspective, using real classes types is safer, expecially in larger applications. If a class get refactored, you get a direct error instead of simply having your callback not called. I think this is especially important since if you use a Regex, and for some reason it doesnt get triggered, this could have severe security implications for the server (aka, Auth no longer being enforced ) I would recommend the following: class SensitiveObject extends Parse.Object {}
class SomeOtherObject extends SensitiveObject {}
Parse.Cloud.beforeSave(Parse.Object, () => console.log("Object saved") )
Parse.Cloud.beforeSave(SensitiveObject, () => throw "Yikes, Not authorized ...." ) On the server side, it's now very easy and simple to locate triggers. triggers.filter( myTrigger => myTrigger.className instanceof className ) |
I think this is a different topic. That is a developer responsibility. Regex has no type safety, it works on a lower level if you will. We cannot force a developer to write tests for their app, nor can we force a developer to only centrally subclass custom Parse Objects instead of writing multiple lines of |
As discussed above, this will allow to have multiple triggers for one class, which is great! Using this method, It would also be fairly easy to guarantee the execution order. Once you filtered the triggers to get only relevant ones. You can in a second step sort the triggers by hineritance order ( aka, the most top object level first ) In my example above, this would result in SomeOtherObject instanceof SensitiveObject && SomeOtherObject instanceof Parse.Object
SensitiveObject instanceof Parse.Object Its now easy to deduce we need to execute them in the following order:
This would solve so many issues :) And simplify cloud functions a lot :) |
Interesting idea. The execution order seems arbitrary. If we go with inheritance principles, a trigger would be inherited by child and we would offer something like a That means one could define a trigger for I think this goes beyond this issue of "wildcard classname in cloud triggers" and requires discussion in a separate issue, if we want to pursue that alley. |
Yes probably. 😄
I completely agree with you on this, but you need to keep in mind that the more options we give the developer, the more chances someone have to make mistakes. Using straightforwoard exact classes names have a very low risk of making mistakes. using
I think that if inheritance is used, each trigger should be independent and un-releated. Ill give you a quick example: You add If for some reason you do not want to have the Another option could be to have DOM-like helper added to the |
Our philosophy is to not add restrictions for all developers of the basis of potential mistakes by some developers. We always aim for versatility. There are other ways (warnings, guidance, etc.) to "help" developers to not make mistakes.
You would need to refactor. It seems arbitrary to execute triggers on an inheritance order, it may work for some use cases, not for others. Using a holistic inheritance mechanism for triggers is likely the most intuitive and universal approach. |
New Feature / Enhancement Checklist
Current Limitation
Currently user have to define cloud triggers one by one. If we have a recurring logic inside of cloud trigger, defining triggers become too hard.
Feature / Enhancement Description
We should be able to define one trigger that applies to all class. Unless that class has its own triggers.
Example Use Case
Alternatives / Workarounds
No known workaround or alternative.
3rd Party References
Its similar to setting SSL sertificates to multiple subdomains.
The text was updated successfully, but these errors were encountered: