Fix REST API for RepositoryInfoResource

 






 

 


  Review Request #1631
  —
  Created June 1, 2010 and submitted

 


















 

  

   Information

  





 
 

  farktronix
 









 
 

  Review Board
 









 
 

  
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
Accessing a repository's info through the REST API was resulting in an exception due to a missing argument to get_object(). 
RepositoryInfoResource was also missing some required variables that get_object() needed.

 








  


 
 

  
Added a unit test for RepositoryInfoResource

 







 









 





  








 









 






  

 








 














 





 

  

   




mike_conley




   

   





  

 



 

  


   

   

    
 
mike_conley


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Nice work - just one note; see below.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/webapi/resources.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    I could be wrong here (still new myself to the WebAPI code), but since Repository is not actually a child of another resource, I think we can omit this line.  

I made the change on my end, still seems to work alright.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      I added this based on this line in the djblets documentation under "Parents and URLs" in djblets/webapi/resources.py:

"Child objects should set the ``model_parent_key`` variable to the field name of the object's parent in the resource hierarchy."

RepositoryResource has repository_info_resource in its item_child_resources array, so I tried to do what other item_child_resource objects did with their model_parent_key value. The value here probably doesn't matter because get_parent_object( ) is never called. Honestly, it was just a stab in the dark, so hopefully chipx86 can clear things up here.
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      Gah- after re-reading my comment and your comment it makes sense. This is talking about the Model hierarchy and not the Resource hierarchy. 
You're right: omitting this line should be fine.
      
 
      

      3. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 





 

  

   




chipx86




   

   





  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/webapi/resources.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    Yeah, this probably isn't needed.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/webapi/resources.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    Actually, this should probably do:

repository = repository_resource.get_object(request, *args, **kwargs)

Then we don't need the model above or the uri_object_key. This file would only have a single line.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 






  4. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/webapi/tests.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    This should use the same format as other unit tests for dynamic parts of the path: "<id>"
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 






  5. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/webapi/tests.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    I'd prefer not hard-coding the number for this test. Instead, do a query first for a Repository entry using the SVNTool. Then plug that ID in and reuse that repository for the check below. That way, if the fixtures change and the Repository we can test with has a new ID, we're not broken by it.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    6. 






  6. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/webapi/tests.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    The parameters should be aligned. They can both be on their own lines, indented 3 spaces from "self.assertEqual"
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    7. 




  7. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    8. 




  

 
















 



 

  

   





 FA





   

   

  

 



 















 





 

  

   




mike_conley




   

   





  

 



 

  


   

   

    
 
mike_conley


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Jacob:

Just one last small thing from me - see below,

-Mike
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/webapi/tests.py
       


        (Diff revision 2)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    Actually, since RepositoryResourceTests inherits from BaseWebAPITestCase, I think you get the repo for free with self.repository. (See reviewboard/webapi/tests.py lines 36 - 39).

So you can omit this line, and below:

rsp = self.apiGet(... % self.repository.pk)
...
self.assertEqual(rsp['info'], self.repository.get_scmtool().get_repository_info())

Tested it locally, seems to work.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   





 FA





   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  









  

   

    Change Summary:
   

   

    
Integrating Mike's feedback.

   

  








  

   

    Diff:
   

   

    
 Revision 3 (+8 -1)
Show changes
 
    reviewboard/webapi/resources.py 
    reviewboard/webapi/tests.py 
 

   

  












  

 
















 





 

  

   




mike_conley




   

   







  

 



 

  


   

   

    
 
mike_conley


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Looks good to me.  Nice work.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Looks good. Committed to master (431d025)
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3.