Pulp

Refactor #3555

Updated by dalley over 6 years ago

In app/viewsets/base.py, we currently have the following "helper" mixins: have: 

 <pre><code class="python"> 
 class NamedModelViewSet(mixins.CreateModelMixin, 
                         mixins.RetrieveModelMixin, 
                         mixins.DestroyModelMixin, 
                         mixins.UpdateModelMixin, 
                         mixins.ListModelMixin, 
                         GenericNamedModelViewSet): 
     """ 
     A viewset that provides default `create()`, `retrieve()`, `update()`, `partial_update()`, 
     `destroy()` and `list()` actions. 
     """ 
     pass 


 class CreateDestroyReadNamedModelViewSet(mixins.CreateModelMixin, 
                                          mixins.RetrieveModelMixin, 
                                          mixins.DestroyModelMixin, 
                                          mixins.ListModelMixin, 
                                          GenericNamedModelViewSet): 
     """ 
     A customized NamedModelViewSet for models that don't support updates. 

     A viewset that provides default `create()`, `retrieve()`, `destroy()` and `list()` actions. 
     """ 
     pass 


 class CreateReadNamedModelViewSet(mixins.CreateModelMixin, 
                                   mixins.RetrieveModelMixin, 
                                   mixins.ListModelMixin, 
                                   GenericNamedModelViewSet): 
     """ 
     A customized NamedModelViewSet for models that don't support updates or deletes. 

     A viewset that provides default `create()`, `retrieve()`, and `list()` actions. 
     """ 
     pass 

 class CreateReadAsyncUpdateDestroyNamedModelViewset(mixins.CreateModelMixin, 
                                                     mixins.RetrieveModelMixin, 
                                                     mixins.ListModelMixin, 
                                                     AsyncUpdateMixin, 
                                                     AsyncRemoveMixin, 
                                                     GenericNamedModelViewSet): 
     """ 
     A viewset that performs asynchronous update and remove operations 

     This viewset should be used with resources that require making a reservation for a repository 
     during an update or delete. 
     """ 
     pass 
 </code></pre> 

 I propose deleting all of them, and renaming the "GenericNamedModelViewSet" to "NamedModelViewSet" for brevity. 

 Rationale: 

 * The name "CreateReadAsyncUpdateDestroyNamedModelViewset" makes me want to burn my eyes out 
 * It's not scalable 
 * These helper mixins make it *too* easy to gloss over the individual actions functions being provided 

 I found a bug [0] where Worker models were able to be created and updated via at the REST endpoint because the viewset inherited from the NamedModelViewSet "helper", which inherited from CreateModelMixin and UpdateModelMixin. 

 Each and every viewset should enumerate all of its provided actions the things it needs to do in a clear way.    These helper mixins don't do that.    Sure, it would make the viewset code a bit "uglier", but there's good ugly and there's bad ugly, and this is bad ugly IMO. 

 [0] https://github.com/pulp/pulp/pull/3413

Back