Utflykten fortsätter med validering av de inmatade värdena, N/S och E/W delarna av en koordinat och avslutas med översättning av felmeddelanden
En sak som boken PHP in Action fäster stor vikt vid är validering av indata. Anledningen till att detta är så viktigt är att det är en grundförutsättning för att minimera säkerhetsproblem med skadlig indata. Man skiljer på client-side och server-side validering. Validering på klientsidan görs direkt i webbläsaren med t ex JavaScript. Syftet med valideringen är att ge användaren snabb återkoppling och upplysa om fel i de inmatade värdena. Däremot kan den inte förhindra skadlig indata. För detta krävs validering på serversidan.
Jag valde att ignorera klientsidan och istället koncentrera mig på serversidan. Det verkade enklast att lägga till valideringen i Http_Request eftersom det den som hanterar indatan. (Ett liknande upplägg hittar man i PHP in Action.) Istället för att komma åt den icke-validerade datan via get()-metoden, så krävs det att den är validerad innan den blir tillgänglig. Detta implementeras med två vektorer i Http_Request. En för den icke-validerade (råa) datan och en för den validerade datan. Själva valideringen görs genom att anropa validate()-metoden och ange fält samt ett valideringsobjekt. Värdet hämtas från den råa datan och, vid godkänd validering, läggs till i den validerade datan.
Jag döpte om den nuvarande get()-metoden till getForValidation() för att på sätt lättare kunna använda de existerande enhetstesterna. Med det namnet finns det ingen risk att man kommer åt icke-validerad data av misstag. Själva testen i sig blev enkla tack vare ett skräddarsytt valideringsobjekt. För utom att kunna ange resultatet av valideringen så kan det även tala om vilka parametrar det blivit anropat med. Det hade gått lika bra med ett s.k. mockobjekt (mer om det senare).
<?php
require_once('simpletest/autorun.php');
class Http_RequestTestsSimpleValidator
{
public $retval;
public $validateCalled = false;
public $validateValue;
public function __construct($retval)
{
$this->retval = $retval;
}
public function isValid($value)
{
$this->validateCalled = true;
$this->validateValue = $value;
return $this->retval;
}
}
class Http_RequestTests extends UnitTestCase
{
...
function testGetReturnsDefaultValueIfNotPostIsValidated()
{
$_POST['key5'] = '7';
$request = new Http_Request();
$this->assertEqual('5', $request->get('key4', '5'));
}
function testGetReturnsValidatedPostValue()
{
$_POST['key6'] = '8';
$request = new Http_Request();
$request->validate('key6', new Http_RequestTestsSimpleValidator(true));
$this->assertEqual('8', $request->get('key6', '5'));
}
function testGetReturnsDefaultIfInvalidPostValue()
{
$_POST['key7'] = '8';
$request = new Http_Request();
$request->validate('key7', new Http_RequestTestsSimpleValidator(false));
$this->assertEqual('5', $request->get('key7', '5'));
}
function testValidatesDefaultValue()
{
$request = new Http_Request();
$validator = new Http_RequestTestsSimpleValidator(true);
$request->validate('key8', $validator);
$this->assertTrue($validator->validateCalled);
$this->assertEqual(false, $validator->validateValue);
}
function testValidateReturnsResultFromValidator()
{
$request = new Http_Request();
$this->assertTrue($request->validate('key7', new Http_RequestTestsSimpleValidator(true)));
$this->assertFalse($request->validate('key7', new Http_RequestTestsSimpleValidator(false)));
}
}
?>
Den uppdaterade versionen av Http_Request:
class Http_Request
{
private $raw_values = array();
private $valid_values = array();
public function __construct()
{
if (!empty($_POST))
$this->raw_values = $_POST;
else if (!empty($_GET))
$this->raw_values = $_GET;
}
public function get($key, $defaultValue = '')
{
return self::getValue($this->valid_values, $key, $defaultValue);
}
public function getForValidation($key, $defaultValue = '')
{
return self::getValue($this->raw_values, $key, $defaultValue);
}
static private function getValue($values, $key, $defaultValue)
{
if (array_key_exists($key, $values))
return $values[$key];
return $defaultValue;
}
public function validate($key, $validator)
{
$value = $this->getForValidation($key);
if ($validator->isValid($value))
{
$this->set($key, $value);
return true;
}
return false;
}
public function set($key, $value)
{
$this->valid_values[$key] = $value;
}
}
Med valideringsmekanismen på plats var det dags att tillämpa den. Men den exakta vägen till slutresultatet är inte speciellt intressant. Det blev ett antal olika test i stil med detta (createRawRequest() är en hjälpmetod som initierar ett Http_Request-objekt med icke-validerad data):
function testNegativeLatDegIsNotValid()
{
$request = $this->createRawRequest('-1', '33.456', '031', '45.123');
$presenter = new Coordinate_Presenter($request);
$this->assertFalse($presenter->validate());
$this->assertFalse($request->get(Coordinate_Presenter::lat_deg));
}
Som synes valde jag att testa valideringsobjekten indirekt via Coordinate_Presenter. Även om det hade varit enklare att testa valideringsobjekten direkt, så hade problemet med att verifiera att presentatören använde rätt valideringsobjekt kvarstått. Koordinaten (0,0) betraktas av det existerande system som ogiltig, så även detta behövde valideringen hantera.
class Coordinate_Presenter
{
...
public function validate()
{
$validators = $this->getValidators();
$this->valid = true;
foreach ($validators as $key => $validator)
{
if (!$this->request->validate($key, $validator))
$this->valid = false;
}
if ($this->getCoordinate() == new Coordinate_Coordinate(0, 0))
$this->valid = false;
return $this->valid;
}
private function getValidators()
{
return array(self::lat_deg => new Validator_Integer(0, 90),
self::lat_min => new Validator_Real(0, 59.999, '{1,2}', '{1,3}'),
self::lon_deg => new Validator_Integer(0, 180),
self::lon_min => new Validator_Real(0, 59.999, '{1,2}', '{1,3}'));
}
}
Det blev två olika valideringsklasser, en för heltal och en för reella tal. Själva grunden för valideringen använder reguljära uttryck, därav de till synes konstiga parametrarna till Validator_Real. Och givetvis blev det ett antal test för respektive valideringsklass.
Om en angiven koordinat är giltig så ska detta visas för användaren. Rent konkret innebär det att utöka vyns gränssnitt med ytterligare en metod, setError(). Presentatörens uppgift är att göra det enkelt för vyn. I det här fallet innebär det att ge vyn det felmeddelande som ska visas för användaren. Men vilket meddelande då, ett på svenska eller på engelska? Och om man hårdkodar meddelandet i testet så innebär det att testet fallerar den dag man ändrar felmeddelandet, vilket inte är bra. Som så ofta när man TDD:ar så börjar man att leta efter en tillräckligt enkel lösning på problemet. I mitt fall innebar det att endast verifiera att setError() blev anropad, men inte fästa någon vikt på själva meddelandet. (Jag återkommer till problemet senare.)
function testPrepareDoesNotSetErrorIfValid()
{
$request = $this->createRawRequest('12', '33.4', '031', '45.123');
$presenter = new Coordinate_Presenter($request);
$this->assertTrue($presenter->validate());
$presenter->prepare($this);
$this->assertFalse($this->setErrorCalled);
}
function testPrepareSetsErrorIfNotValid()
{
$request = $this->createRawRequest('12', '-33.4', '031', '45.123');
$presenter = new Coordinate_Presenter($request);
$this->assertFalse($presenter->validate());
$presenter->prepare($this);
$this->assertTrue($this->setErrorCalled);
}
Det som inte visats här är när själva valideringen utförs av sidan. Det sker när sidan postas tillbaka. Resultatet av valideringen styr om värden i databasen ska uppdateras eller om sidan ska visas igen med felmeddelanden. Detta sker för samtliga fält på sidan. Det ligger nära tillhands med en gemensam basklass för samtliga presentatörer, samt en lista i sidan som innehåller presentatörerna. Valideringen skulle då kunna reduceras till en for-loop.
Att lägga till hantering av N/S och E/W innebar inga större överrakningar. Valideringen är enkel. Endast 'N' eller 'S' är tillåten indata. Däremot blev omvandlingen till koordinat lite mer komplicerad. Det görs via en statisk metod fromDegMin() på Coordinate-klassen som tar in grad (mellan -90 och +90 för latitud) och minut. Metoderna för att beräkna grad för latitud respektive longitud ser ut som följer.
private function getLatDeg()
{
$deg = $this->coordinate->latDeg();
$lat_hem = $this->request->get(self::lat_hem, self::getLatHemString($deg));
$lat_deg = $this->request->get(self::lat_deg, abs($deg));
return ($lat_hem == 'N') ? $lat_deg : -$lat_deg;
}
private function getLonDeg()
{
$deg = $this->coordinate->lonDeg();
$lon_hem = $this->request->get(self::lon_hem, self::getLonHemString($deg));
$lon_deg = $this->request->get(self::lon_deg, abs($deg));
return ($lon_hem == 'E') ? $lon_deg : -$lon_deg;
}
Kod som är så lika tyder i stort sett alltid på att det finns någon likhet man kan ta tillvara på och använda för att förenkla. Spontant kan jag tänka på två olika sätt att hantera det. Antingen modifierar man fromDegMin() till att även ta in N/S i någon tappning och låter den hantera positivt/negativt värde. Eller så delar man upp den nuvarande presentatören i två delar. En för latitud och en för longitud. Ser man till symmetrin i Coordinate_Presenter så känns det som det bästa alternativet.
Självklart innebar det också ändringar i Coordinate_View. Förutom att de två valen N/S ska visas så ska även rätt värde vara valt på förhand. Detta gör att den HTML som vyn levererar inte är helt statisk. Det föll sig naturligt att lägga till en liten hjälpmetod som hanterade det.
class Coordinate_View
{
private $lat_hem;
private $lat_deg;
private $lat_min;
private $lon_hem;
private $lon_deg;
private $lon_min;
private $error;
public function setLatitude($hem, $deg, $min)
{
$this->lat_hem = $hem;
$this->lat_deg = $deg;
$this->lat_min = $min;
}
public function setLongitude($hem, $deg, $min)
{
$this->lon_hem = $hem;
$this->lon_deg = $deg;
$this->lon_min = $min;
}
public function setError($error)
{
$this->error = $error;
}
public function getHtml($presenter)
{
$presenter->prepare($this);
$html = '<table class="input_coord">
<tr>
<td>
<select name="' . Coordinate_Presenter::lat_hem . '" class="input_coord_hem">
' . self::getOption('N', $this->lat_hem) . '
' . self::getOption('S', $this->lat_hem) . '
</select>
</td>
<td>
<input type="text" name="' . Coordinate_Presenter::lat_deg . '" maxlength="2" value="' . $this->lat_deg . '" class="input_coord_deg" />°
</td>
<td>
<input type="text" name="' . Coordinate_Presenter::lat_min . '" maxlength="6" value="' . $this->lat_min . '" class="input_coord_min"/>\'
</td>
</tr>
<tr>
<td>
<select name="' . Coordinate_Presenter::lon_hem . '" class="input_coord_hem">
' . self::getOption('E', $this->lon_hem) . '
' . self::getOption('W', $this->lon_hem) . '
</select>
</td>
<td>
<input type="text" name="' . Coordinate_Presenter::lon_deg . '" maxlength="3" value="' . $this->lon_deg . '" class="input_coord_deg" />°
</td>
<td>
<input type="text" name="' . Coordinate_Presenter::lon_min . '" maxlength="6" value="' . $this->lon_min . '" class="input_coord_min" />\'
</td>
</tr>';
if ($this->error)
$html .= '<tr><td>' . $this->error . '</td></tr>';
$html .= '</table>';
return $html;
}
static private function getOption($value, $selected_value)
{
$selected = ($value == $selected_value) ? ' selected=""' : '';
return '<option value="' . $value . '"' . $selected . '>' . $value . '</option>';
}
}
Jag har medvetet låtit bli att göra för mycket med vyn. Mest för att jag inte är övertygad att det är ett bra sätt att lösa problemet på. Det bästa vore om allt som har med användargränssnittet att göra låg i en mallfil. Men om man skulle gå vidare så är det uppenbart att mycket av HTML:en skulle man kunna generera dynamiskt. En fördel med det är att det skulle minska risken för fel i HTML:en.
En annan sak som jag inte är nöjd med är den lösa kopplingen från vy till presentatör. En ändring i antingen den ena eller den andra skulle lätt kunna leda till att de inte längre fungerar tillsammans. Ett sätt att hantera det på skulle kunna vara att testa hela webbsidan. Och då kanske det till och med skulle vara en fördel med nuvarande sätt att generera HTML:en på eftersom man har den relevanta HTML:en lättillgänglig. Jag har ännu inte undersökt möjligheterna att testa kompletta sidor med SimpleTest, men det finns anledning att göra det.
Jag lovade att återkomma till problemet med felmeddelandet och verifiering att det faktiskt blir korrekt. En sak som jag tycker är mycket användbart i samband med TDD är mockobjekt. Ett sådant objekt ser ut som och uppför sig som ett riktig objekt, men dess syfte är att verifiera att objektet som använder den gör det på rätt sätt.
I vårt fall vill vi att Coordinate_Presenter anropar ett objekt som hanterar översättning av texter och begär texten för ogiltig koordinat, samt skickar den texten till vyn. Detta gör vi genom att skapa upp ett mockobjekt som ser ut som översättningsobjektet. Vi talar om för objektet att det ska förvänta sig ett anrop där ogiltig koordinat efterfrågas, och då ska den skicka tillbaka en specifik sträng. När testfallet sedan körs så verifieras att mockobjektet blir anropad på förväntat sätt.
function testErrorMessageIsTranslated()
{
$translator = new MockLanguage_Translator();
$translator->setReturnValue('translate', 'Coordinate Error');
$translator->expectOnce('translate', array('bad_coordinates'));
$request = $this->createRawRequest('N', '0', '0', 'E', '0', '0');
$presenter = new Coordinate_Presenter($request, $translator);
$this->assertFalse($presenter->validate());
$presenter->prepare($this);
$this->assertEqual('Coordinate Error', $this->errorMessage);
}
Det vi gör nu är att verifiera ett visst beteende utan att bekymra oss om implementationsdetaljer. Ofta är det bra att kunna flytta på problem på det här sättet. Exakt hur översättning ska implementeras är ointressant så länge som den uppvisar det beteende vi har "specificerat" i och med mockobjektet. Dessutom slapp vi både språkproblemet och hårdkodning av felmeddelandet.
PHP land är stort och, för min del, outforskat, så det blir nog tillfällen för fler utflykter.
Inga kommentarer:
Skicka en kommentar