Leren van de fouten van Coconut
or: How did we manage to fuck up this time?
Wie ben ik?
-
Naam: René van den Berg
-
Functie: Coconut backend-ontwikkelaar
-
Tags: security, ruby, rails, ruby on rails, beautifulcode
-
Weet ook wel wat van: javascript, emberjs, jquery,
-
front-end-ontwikkeling
-
Special: Level 4 algemeen criticus
Coconut: geschiedenis
Ooit (eind 2008) begonnen als OGD Intranet v4
( Rails 2.2, Ruby 1.8.6 )
In eerste instantie Terminal
In 2011 rebranded tot Coconut
Huidige team inmiddels allemaal minimaal 3 jaar actief
Coconut: ontwikkeling in het kort
-
Scrum
- Rails 3.2, Ruby 1.9.2, MySQL, Redis, Memcached, ...
- Functioneel ontwerp
- Interactie, grafisch etc... wanneer nodig
- Pair programming
- Unit- en andere technische tests
- Continuous Integration
- Code Reviews
- Reviews
- Staging:
- alpha
- beta
- production
- verschillende unieke klantomgevingen
Twee soorten fouten
Procesfouten
en technische fouten
Procesfouten:
planning, things-that-just-happen, coördinatie, hiring mistakes, etc
Technische fouten:
failure to refactor, failure to test, failure to release, accepting broken tests, breaking release paths...
Veel technische fouten vinden hun oorzaak in procesfouten!
Procesfout: twijfel
Als je het even niet weet, maak je een configuratie-optie. Als die overal hetzelfde wordt ingesteld, is het geen configuratie-optie!
Procesfout: OGD, OGD, OGD
Terminal werd ooit gebouwd voor OGD.
En dus: (dit is een model!)
Procesfout: first release syndrome
-
De eerste release liet te lang op zich wachten...
-
... en dus werd er een deadline ingesteld (fixed time)...
-
... en kon er niet meer gepraat worden over
welke features er wel en niet noodzakelijk waren (fixed features).
De enige flexibele dimensie die je dan overhoudt is 'quality'
And boy, do we still feel that today.
Procesfout: keuze van developers
Iedere softwareontwikkelaar die op de bank zat, mocht aan de slag bij Coconut.
Zonder ervaring met Ruby.
Zonder ervaring met Rails.
Zonder ervaring met unit tests.
Zonder ervaring met webapplicatie-ontwikkeling.
Maar met het idee dat ze elk moment van de bank, en dus van de klus af gehaald konden worden.
Procesfout: 80 / 20
Als je 80 procent van de features kan bouwen...
... in 20 procent van de tijd ...
... check dan of die andere 20 procent essentieel is.
Zo ja: bouwen.
Zo nee: vooruitschuiven of afschaffen.
Maar: 80 % van de vereiste features bouwen is niet acceptabel.
Procesfout: itereren over designs...
... in plaats van itereren over versies.
In theorie zijn theorie en praktijk het zelfde - in de praktijk valt dat tegen.
Na drie versies van een design weet je niet hoe het product werkt.
Na een eerste versie van de code wel. Dus bouw dat prototype.
Procesfout: denken dat je later de tijd gaat krijgen om iets te fixen
Die krijg je niet.
En 'the big rewrite in the sky' komt niet.
En daar mag je blij mee zijn (Spolsky).
Modularisatie
Terminal was nooit bedoeld om losse modules aan en uit te zetten
Alle mogelijkheden die we nu hebben in die richting zijn hacks. Steeds minder smerige hacks, maar dan nog steeds hacks.
Als je niet modulair bouwt, is het bijna onmogelijk om het later alsnog modulair te maken.
Failure to refactor
The first time you do something, you just do it. The second time you do something similar, you wince at the duplication, but you do the duplicate thing anyway. The third time you do something similar, you refactor.
Tip:
Three strikes and you refactor.
Notifications helper (1)
Notifications helper (2)
Notifications helper (4)
Notifications helper (5)
Notifications helper (8)
-
begon met 5 cases en 40 regels
-
eind 2009 ~10 cases, waarvan 3
met nested logica, 80 regels
-
inmiddels 225 regels:
-
javascript-rendering
-
error-handling
-
html concatenation
-
escaping
-
etc
Geldt allemaal net zo sterk voor User model (243 regels) en Profile (320 regels). Die laatste heeft ook nog een lading runtime-gegenereerde code.
Wanneer hadden we moeten refactoren?
Elke keer als we hier iets aan toevoegden! Een feature, een notificatie, of wat dan ook.
Vertalingen
Een eerste versie waar de teksten hardcoded worden in de overige code is niet acceptabel!
Code Consumption
Onze code leek voor ons alleen te zijn...
... API's en REST waren niet zo belangrijk ...
... en routes waren ook gewoon code, dat kunnen we wel veranderen ...
Permalinks? ehhh...
Chrome plugins? Oh joh... ehhh...
Een mobiele app? Laat me even denken...
Je framework lost je probleem niet voor je op
Je JS-MVC-Framework trouwens ook niet.
Je standaard keurige MVC-structuur gaat omvallen.
Ieder patroon dat je loslaat op welke code dan ook heeft problemen bij het toepassen in de praktijk (100% REST kan bijvoorbeeld meestal ook niet).
Om je Framework heenwerken, daarentegen, gaat je problemen alleen maar erger maken.
Security (1)
Webdevelopers die niets weten van security zijn een probleem.
ECHT
Security (2)
XSS: sanitization, maar dan op het verkeerde moment
Coconut sanitizet data voordat die de database in gaat. Dat levert nog wel eens problemen op met doubly sanitized HTML. In die tijd was er nog niets wat output escaping op de juiste manier aanbood (inmiddels doet Rails dat zelf).
Security (3)
Authorizatie
Gebeurde heel vroeger door code uit de database te evallen en te kijken of er true of false uit kwam.
En toen bleek het database veld maar 256 karakters lang te zijn
En dat gaat vroeger of later fout (bijvoorbeeld een &&-clause die wegvalt)...
De boodschap: zorg dat je iemand hebt die Security snapt!
Architectuur
In veel gevallen gewoon vertrouwd op Framework zonder het helemaal te snappen. Resultaat:
(onder meer) onduidelijke 1-op-1 relaties tussen gebruikers en profielen.
-
Wie heeft er nou een avatar?
-
Een telefoonnummer?
-
Een e-mailadres?
Gebrek aan documentatie
Code is read more often than it is written. It’s true.
Soms bijna dezelfde methode twee keer gemaakt
“Tests as documentation” makes me laugh. Zeker als je er meer dan 5000 hebt.
Hoe komt het dat jullie nog kunnen ontwikkelen?
Nou, we hebben ook wat dingen goed gedaan:
boyscout-regel
code reviews, CI, etc
FYFBD
Tests, inclusief integration (Javascript needs work)
Just one more thing
Coding standards hebben wij niet nodig. Maar de BUS wel.