- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 4.8k
 
Endpoint for purging all objects in class #2032
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
Conversation
          Current coverage is 91.29%@@             master      #2032   diff @@
==========================================
  Files            92         93     +1   
  Lines          6517       6537    +20   
  Methods        1161       1169     +8   
  Messages          0          0          
  Branches       1368       1370     +2   
==========================================
+ Hits           5943       5968    +25   
+ Misses          574        569     -5   
  Partials          0          0          
  | 
    
| }); | ||
| } | ||
| 
               | 
          ||
| purgeCollection(name: string) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have deleteObjectsByQuery which will remove all objects if you pass {} as the query. Can you use that instead?
| 
           @Marco129 updated the pull request.  | 
    
    
      
        1 similar comment
      
    
  
    | 
           @Marco129 updated the pull request.  | 
    
        
          
                src/rest.js
              
                Outdated
          
        
      | }).then(() => { | ||
| if (className == '_Session') { | ||
| var cacheAdapter = config.cacheController; | ||
| cacheAdapter.user.clear(); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to do something similar for role cache. Probably also worth adding a test to make sure that deleting all objects actually causes the cache to get cleared (eg. create a role, add a user to it, delete all roles using this endpoint, and ensure that the user no longer has access to objects that only that role could access)
| 
           Needs a few changes, which I see you are already starting on :) You should join our development slack channel, shoot me an email (see my profile) and I can add you.  | 
    
| 
           @Marco129 updated the pull request.  | 
    
| 
           @Marco129 updated the pull request.  | 
    
| 
           Seems like tests are failing now :( maybe something you were relying on changed?  | 
    
| 
           They passed on local :( will take a look again  | 
    
| 
           Might be fixed by #2038.  | 
    
| 
           @Marco129 updated the pull request.  | 
    
| 
           Yeah , the failures look like the exact same symptoms as the deepcopy 0.6.1 symptoms. Rebasing onto master should fix the issue.  | 
    
| 
           @Marco129 updated the pull request.  | 
    
| 
           @Marco129 updated the pull request.  | 
    
| 
           Looks good to me. Since this involves a new public API, I'm wait a bit more to see if anyone else has any comments.  | 
    
| 
           if someone screws up a delete request and posts   | 
    
| 
           That is actually a really good point. The endpoint does require the master key, but you could be deleting an object using buggy cloud code, where the master key is expected to be used, and also you may end up with an empty string there accidentally.  | 
    
| 
           In the old Parse.com API we would use something like  What about   | 
    
| 
           reasonable compromise! /purge/++  | 
    
| 
           Why not on the schema endpoint then?  | 
    
| 
           Purge endpoint makes sense  | 
    
| 
           Why do we limit this by master key, could we not allow a user to delete all their data?  | 
    
| 
           @Marco129 updated the pull request.  | 
    
    
      
        1 similar comment
      
    
  
    | 
           @Marco129 updated the pull request.  | 
    
| 
           Cool, I think we've reached a good consensus. If people have other ideas, we can still change this before we push the next release. @blacha allowing a user to delete all data they have write access for seems a little dangerous to me. I think we would want to restrict to classes they also have find permission on, as people may be protecting objects by preventing their object ID from getting out. I think it could be a good idea, but needs more thought.  | 
    
Special master key only endpoint related to parse-community/parse-dashboard#124
Usage:
DELETE /classes/<className>