Rails anti patterns

A loose collection of common bad practices in Rails projects

Transcript

1. Rails anti patterns Florent Guilleux, Ruby Peru meetup, April 10th 2014 1 / 13
2. Disclaimer Not exhaustive list, not always Rails specific... 2 / 13
3. Storing credentials in versioned files If your source code is accessed by an attacker, people may use your credentials: for their own benefits (S3) send emails on your behalf (SendGrid) harm your reputation (Twitter) impersonate users (s e c r e t _ k e y _ b a s e ) erase data (S3) ... So store your credentials in ENV variables. 3 / 13
4. Using different database engines in development and production DBs engines have differences, what works with your SQLite in development may not with the PostgreSQL in production. For instance PostgreSQL follows more closely the SQL standard for the G R O U P B Y clause. 4 / 13
5. Anti patterns in views write DB calls misuse of h t m l _ s a f e < % = " < h 1 > # { t e x t } < / h 1 > " . h t m l _ s a f e % > # t e x t i s n o t e s c a p e d ! # s h o u l d b e d i r e c t l y < % = c o n t e n t _ t a g : h 1 , t e x t % > See Everything you know about html_safe is wrong overuse of view helpers < % = l i n k _ t o " A n o t h e r w e b p a g e " , " h t t p : / / a n o t h e r p a g e . c o m " % > # s h o u l d b e d i r e c t l y < a h r e f = " h t t p : / / a n o t h e r p a g e . c o m " > A n o t h e r w e b p a g e < / a > 5 / 13
6. Muti anti patterns combo c l a s s P o s t < A c t i v e R e c o r d : : B a s e a f t e r _ c r e a t e : p o s t _ t o _ t w i t t e r p r i v a t e d e f p o s t _ t o _ t w i t t e r t w i t t e r _ c l i e n t . u p d a t e " N e w p o s t : # { c o n t e n t } " e n d e n d 6 / 13
7. Muti anti patterns combo c l a s s P o s t < A c t i v e R e c o r d : : B a s e a f t e r _ c r e a t e : p o s t _ t o _ t w i t t e r p r i v a t e d e f p o s t _ t o _ t w i t t e r t w i t t e r _ c l i e n t . u p d a t e " N e w p o s t : # { c o n t e n t } " e n d e n d What's wrong here: no handling of the possible failures of the external API call if the call to the Twitter API fails (for instance Twitter is unavailable), the P o s t record is not created tweeting is probably not a core responsibility of the P o s t class P o s t instance creation is unnecessarily slow 7 / 13
8. Not dealing correctly with possible failures of external APIs Calls to external APIs may fail to a variety of reasons (incorrect params, API change, call timeout, network issue, etc.). You need to handle gracefully possible errors (it's not easy to find out all the exception classes you need to catch...) d e f p o s t _ t o _ t w i t t e r t w i t t e r _ c l i e n t . u p d a t e " N e w p o s t : # { c o n t e n t } " r e s c u e T w i t t e r : : E r r o r , T i m e o u t : : E r r o r , E r r n o : : E C O N N R E S E T # , e t c . # h a n d l e t h e e r r o r e n d 8 / 13
9. Putting in a callback a non core responsibility of a class creates a new dependency in the class harder to grasp the full behavior of the class make instance creation/update/destroy slower can't easily create a record without side effects 9 / 13
10. Some alternatives to callbacks Put the call to Twitter in the controller (either directly or through a service object): c l a s s P o s t s C o n t r o l l e r < A p p l i c a t i o n C o n t r o l l e r d e f c r e a t e . . . i f @ p o s t . s a v e t w i t t e r _ c l i e n t . u p d a t e " N e w p o s t : # { @ p o s t . c o n t e n t } " r e d i r e c t _ t o @ p o s t e n d e n d Or, if posting to Twitter is a core responsibility of the class, decorate the native s a v e method: d e f s a v e _ a n d _ b r o a d c a s t t o _ t w i t t e r = f a l s e i f s a v e t w i t t e r _ c l i e n t . u p d a t e ( " N e w p o s t : # { @ p o s t . c o n t e n t } " ) i f t o _ t w i t t e r e n d e n d 10 / 13
11. Good practice: use a callback only when the logic refers to state internal to the object source: "The only acceptable use for callbacks in Rails ever" 11 / 13
12. Calling external API "inline" We don't know how much time an external API call can take. If we don't need the result of the call, or don't need it immediately, just do the call in a background job. c l a s s P o s t s C o n t r o l l e r < A p p l i c a t i o n C o n t r o l l e r d e f c r e a t e . . . i f @ p o s t . s a v e T w i t t e r P o s t W o r k e r . p e r f o r m _ a s y n c ( @ p o s t . i d ) r e d i r e c t _ t o @ p o s t e n d e n d 12 / 13
13. Last anti pattern: Using Rails 13 / 13