Symfony forms – Flexible widgets based on user credentials (sfcontext is evil)

Background

This issue has come up many times in the symfony forum and on IRC, and whilst it seems like a fairly trivial one, it is important to discuss the best practice around it. Often we want to modify a form based on something outside the form’s scope, like a user’s credentials or the page they are on, or maybe some session values – lets take the example of a dropdown widget where admin users get to see a few more options.

The bad way

Everyone that has been using symfony for a while is aware of the context singleton. This magic sprite allows us to grab all sorts of information about the context we are in, including the user, session, request, view, and many more things. Whilst it certainly has its place, most of the time, however, we should avoid using it. The clue to the reason why is carefully disguised in the class name “Context”.

When we refer to a context in our code, we are locking ourselves in to the fact that the context must exist, so every time we use it we are basically saying that this class can now only be used in a symfony project with a fully initialised symfony stack hanging over it. This becomes a problem in things like unit tests, where you have to mock up a loaded context object with bells and whistles in order to test a simple function or class.

So this is what you might think of doing:

// In your form class
public function configure()
{
  $choices = array(1 => "something boring", 2 => "something dull");
 
  $currentUser = sfContext::getInstance()->getUser();
 
  if ($currentUser->isAuthenticated() && $currentUser->hasCredential("admin"))
  {
    $choices += array(5 => "something cool", 6 => "something leet");
  }
 
  // ....
}

The reason this is bad is that the forms framework is a standalone framework – it should be possible to pick up your form class and drop it into any project. It should also be possible to test it independently of symfony, without being tied in by sfContext. So what is the better way?

The dependency injection approach

You might read that and think “woah, this is getting complicated!” but we’re not talking about dependency injection containers here, we’re simply saying that you can make your form object depend on something to run. The thing it depends on should not be the context singleton, it should be the minimum thing that the form needs to operate correctly – which in this case, is a user object that supports credentials.

// In your actions.class.php
$this->form = new myForm(array(), array("currentUser" => $this->getUser()));
 
// In your form class
 
// ...
if ( !($this->getOption("currentUser") instanceof sfUser))
{
  throw new InvalidArgumentException("You must pass a user object as an option to this form!");
}

The test here is where we are making this form “dependent” on a user object. In this case we are insisting that the object is an instance of sfUser, which you may argue is tying us in to symfony again, but you could use any test here to ensure that the object will have the necessary functionality you need, maybe check for the existence of a “hasCredential()” method for example.

When writing a test for this form class, we now only need to instantiate a user object and load it with some credentials – much easier than doing the same thing and locking into a context singleton. There may be other times when this form could be useful in a lightweight environment, where you can get speedy access to a user object but don’t want the overhead of the symfony context – you might not think of one now, but it’s best to code this way and you’ll have less reasons to kick yourself further down the line.

The completed code, for our single widget form

// In your form class
public function configure()
{
  if (!($this->getOption("currentUser") instanceof sfUser))
  {
    throw new InvalidArgumentException("You must pass a user object as an option to this form!");
  }
  else
  {
    $currentUser = $this->getOption("currentUser");
  }
  $choices = array(1 => "something boring", 2 => "something dull");
 
  if ($currentUser->isAuthenticated() && $currentUser->hasCredential("admin"))
  {
    $choices += array(5 => "something cool", 6 => "something leet");
  }
 
  $this->widgetSchema['my_dropdown']    = new sfWidgetFormChoice(array("choices" => $choices));
  $this->validatorSchema['my_dropdown'] = new sfValidatorChoice(array("choices" => array_keys($choices)));
}

Wrap up

Think about this example the next time you think about modifying a query based on a user object, or session value in a peer class, or Doctrine table class… Maybe you should have passed a parameter there too? Every class and method you write, think about how you can reuse it, will it even be possible the way you have written it? If something simply must be coded in a symfony specific way, think about making a parent and a child class for the problem you are trying to solve. In the parent class, you can make things as generic as possible – so you can re-use that class to your heart’s content. In the child class, you can add the symfony specific code – kept to a minimum.

Doctrine Gotchas – delete cascade, SoftDelete and foreignType

These are mistakes anyone can make, as proved by our team of experienced developers who only came across these late into a project – after developing with Doctrine for over 6 months.

Cascading / Transitive persistence

In Doctrine there are two ways to define a cascade, one will tell Doctrine what to do, the other defines how that condition is set up on your database when Doctrine builds the sql commands. The reason this was a gotcha for me, is that we only ever used one of the types, and were completely unaware of the other one! The Doctrine manual is quite long, and unless you’ve read it several times cover to cover it’s easy to miss these things – although next time I will at least be reading the appropriate section of the manual, regardless of what I think I already know.

So, you want a delete cascade? You might do something like this in your yaml file:

PhoneNumber:
  columns:
    user_id:
      type: integer
  relations:
    User:
      class:       User
      local:       user_id
      foreign:     id
      onDelete:    CASCADE

What the above definition says is that this phone number is connected to a user, and if the user record is deleted – in order to maintain database integrity with foreign keys, the corresponding phone number records should also be deleted. This is perfect behaviour – but Doctrine does not do anything with it, it simply sets up the database to handle it, so if you are using a dbms that supports delete cascades then everything will be handled at the database level.

Adding softDelete to the equation

So here comes the gotcha, all of our tables use the softDelete behaviour – which means that instead of the row being deleted, a flag is set (deleted=true) which then magically makes the row appear deleted in all your queries (Providing you have dql callbacks turned on). This means that the row is never actually deleted at the databse level, and thus the cascade is never applied. Now I always knew this, I mean I wouldn’t install a behaviour before knowing what it does right? What I wrongly assumed was that the call to onDelete: CASCADE would take care of “soft deleting” my relations too, but it doesn’t, in fact that line is effectively useless when combined with softDelete. For that you need to set the doctrine cascade parameter:

PhoneNumber:
  actAs: [softDelete]
  columns:
    user_id:
      type: integer
  relations:
    User:
      class:       User
      local:       user_id
      foreign:     id
      foreignType: one
      cascade:     [delete]

Now the behaviour will be as expected. If you are using softDelete and your application has a lot of delete operations, you should take some time to understand what is happening, because Doctrine must fetch and load all of the related objects and then check their relations and cascade settings and so on, so in a large system deleting a single user could result in quite a lot of database queries and cpu time. It is for this reason that database level cascades are preferable, so maybe being more picky about which models you apply the softDelete behaviour would be prudent. Learn more about Doctrine Transitive Persistance.

Defining types of foreign key relations

This one is also one of those “read the manual” situations, but similarly to the above example what caught us out here is that we again thought that there was only one option for defining what we wanted to do, when in fact there were too parameters we should have been considering. This gotcha also only applies if you are defining your relations in a yaml file, as the problem lies in its interpretation when Doctrine builds the base classes.

Look at the following example:

User:
  columns:
    id:
      type: integer
      notnull: true
  relations:
    Payment:
      class:       Payment
      local:       id
      foreign:     user_id
      foreignType: many

The intention was that a user could have multiple payments, and it seemed to make sense to define it using the foreignType parameter above, however when the model was built – this always resulted in a $this->hasOne(…) statement being created, because what was actually happening is that we were not defining the relationship at the other end, we were actually defining it at the local end! So foreignType: many actually means “Many users can have one payment” – hence the interpretation of $this->hasOne(..)

In actual fact using the foreignType key is mostly useless, as Doctrine “guesses” this side of the relationship. It’s only necessary if Doctrine guesses incorrectly and you need to override it, our example above should actually have been using the “type” parameter, which defines the relationship from the other perspective:

User:
  columns:
    id:
      type: integer
      notnull: true
  relations:
    Payments:
      class:    Payment
      local:    id
      foreign:  user_id
      type:     many

Adding [ foreignType: one ] to this schema declaration would have no offect, as Doctrine would guess it to be the case (in this example). The above declaration will now correctly form a $this->hasMany(…) statement in your base class. To make it easy to remember, just forget about foreignType all together unless you come across a special case where you nee it – and prepend the word “has” to the type parameter, that should give you a clue as to what you are defining.

So, with one-to-one relationships, the type parameter is unnecessary, with one-to-many you specify the type: many (Doctrine will guess the one part) and with many-to-one you also do not need to specify the type as it will be guessed based on how you’ve got the relation set up from the other model’s perspective.

Read more about relations in the Doctrine manual.

Conclusion

it’s hard to believe that after 6 months of coding a large application that currently has over 30,000 lines of code and growing we could have a schema file with several useless calls to onDelete: CASCADE and even more useless calls to foreignType. It’s even harder to believe that in the early days of the project, we simply overrode the setup methods to add the relations that we needed, when the Schema.yml file failed to come up with the goods. Next time I think I’ll spend a little more time with the documentation, although we are of course experts now – so maybe next time we won’t need to? 😉

PHP TestFest Norway 2009

Test Fest Norway 2009

If you use PHP and have ever wondered why you get something so cool for free, maybe a little tang of guilt can creep in. Contributing to the language in one way or another is a great way to pay something back, and there are many ways to do so, one of which is to write tests….

Tests keep the code sane. Every time things change, the tests can be run to make sure that everything still works ok – just like the php tests you (hopefully) write in your own projects. The best way to maintain this stability for the language itself is also to have as much code covered by tests as possible.

Every year, the PHP team organises TestFest, aiming to introduce testing to the masses in the most straightforward way. All you have to do is find out where your nearest event is being held and sign up. You will be given everything you need and assisted in every way to help you write at least one test – hopefully many more.

Details of TestFest 2009 Norway have just been released, if you are in the area, check it out!

Customising Symfony forms – be careful with base class inheritence

Background

Recently I was working on a form for updating a couple of very simple values for a single table. When creating such forms, where we only need a subset of the available columns to be editable, we always have the option of either unsetting the fields we don’t need, or overriding the widgetSchema. In this case, I opted for the latter, since I only needed to edit 2 columns out of a possible 10, I didn’t think adding 8 fields to the unset() function was the cleanest way. The following examples contain obfuscated data.

Overriding the widget schema

public function configure()
{
  $this->setWidgets(array(
    'amount'               => new sfWidgetFormInput(),
    'reduced_amount' => new sfWidgetFormInput(),
    ));
 
   //Labels and decorator stuff here
}

My plan was to inherit the validators that already exist in the base class, since they do the job for what I need.

The error

The form worked fine for an insert, but when I came to update an existing record, the error was quite strange:

SQLSTATE[23505]: Unique violation: 7 ERROR: 
  duplicate key value violates unique constraint 
  "body_fee_version_pkey"

The problem it seemed was that my versionable behaviour was not incrementing the version value before attempting to insert a new version record. After a long period of debugging the versionable behaviour, along with some of my other custom behaviours, I was no closer to a solution.

I started to dig into the form classes, working backwords through all the object update methods, save, dosave, etc. Until I finally stumbled across this line:

$this->values = $this->validatorSchema->clean(
   self::deepArrayUnion($this->taintedValues, 
   self::convertFileInformation($this->taintedFiles))
 );

Before this call, everything seemed ok, but after this call, my values array, which at this stage only contained the two fields that had been posted, now suddenly included a value for all the fields in the table. Why? It then occurred to me that the entire validator schema was being processed, not just the fields that are actually posted! This means that all the validators that are required=false will silently return a “clean” value, which is most likely the database default.

So what did this mean? Well, it meant that the validator was “cleaning” all the columns that had not been submitted with the form, including the version column, which was being set to null. When the versionable behaviour kicked in, it read this null value and incremented by 1 for the next version, which then became 1 – a version which of course already existed, causing the error.

The solution

The solution is blindingly simple, we don’t just declare the widget schema, we must also declare the validator schema. Whilst this seems like it makes sense, I feel that it is a shame that I have to essentially copy and paste the necessary validators from the base class. The alternative of course would have been to unset the offending fields, but then we are back to option 1 above, unsetting 8 of 10 fields when it seems cleaner just to declare the 2 fields I actially need.

  $this->setWidgets(array(
    'amount'               => new sfWidgetFormInput(),
    'reduced_amount' => new sfWidgetFormInput(),
    ));
 
// Messages declared here as array since they are the same
 
$this->setValidators(array(
  "amount"  => new sfNumberValidator(
                       array('required' => true), $messages),
  "reduced_amount" => new sfNumberValidator(
                       array(), $messages),
 ));
 
   //Labels and decorator stuff here

Conclusion

I have been working with Symfony for over a year and a half, and with Symfony forms since they were born – and yet I was still caught out by something that seems quite simple, because I assumed it would be ok. There are so many things that could have alerted me to this problem and saved me a lot of time, for example if any of the extra fields had been required=true then at least I would have had some form errors to give me a clue!

I may suggest that the default behaviour should be to only process the validators that match the widgets, or maybe at least for the form to err if extra validators are found that are not used, as I feel that this is a mistake that others could make, and as I mentioned above, it’s a shame that we have to re-declare validators that are already present in the base class.

Redpill-Linpro at Symfony Camp, Dutch Open Projects 2008

Intro

Last Friday, reading Jonathan Wage’s Doctrine blog, I spotted a post in the top right hand corner which Read “Doctrine at Symfony Camp 2008”, and I thought “Symfony Camp”? Wassat? So I took a look and realised that this would be a great event to attend, as I’m currently between projects and about to move from Symfony 1.0 to Symfony 1.1/1.2.

Working for an open source company (Redpill-Linpro) has it’s benefits – it took around an hour on Monday afternoon to gather interested parties (Hannes was the only one that could join me unfortunately), check with the boss(es), book flights, hire car and register. We were booked and ready to go.

Day 0 – Polar Bears and Pizza

We made it Gardermoen airport in plenty of time, noticing that the helpful monitor on the express train was informing us that our flight was delayed by an hour, giving us chance to play a couple of games of Hedgewars while we waited 🙂

Once we arrived in Amsterdam, I had the amusing pleasure of watching Hannes search in desperation for a place to smoke in a non-smoking airport, we grabbed our bags and then with a newly-chilled Icelander in tow proceeded to wind up the Alamo guy and get a nicely upgraded Volvo S40 hire car.

After a slight tour of the airport, TomTom took us to Amersfoort, where we drew out some €uros and looked in vain for  a place to eat. After giving up on the search (it was around 23:30 by now), we headed off to find the camp. TomTom took us to the correct road, but from that point on we were on our own – after driving down a road with an almost cartoon-like amount of road signs informing us of bends, obstructions, rabbits, etc. we found our headlights pointing down a cycle path. The sign showed a “dead end” symbol (“T” with a red top) and a couple of pictures of bikes an mopeds. On the floor was a dotted white line, separating the “road” into two sections barely big enough for a couple of bikes each. After consulting Hannes, we decided that this was not something we should be driving down, and headed back for a search of the area.

About 15 minutes later we were back driving down the “cycle path”. Yes, to get to Dutch Open Projects you have to drive down a cycle path – which “technically” is also a road, until just after the gate to their offices when a new sign informs you that only bikes may pass.

Hungry from our trip, we were pleased when offered some sandwiches by our new hosts which we munched on the way to setting up our camp beds in the military style tents. Everybody (except Fabien) was still up, so we joined “the Italians” and the beer started flowing.

During the rest of the “evening”, random shouts of “Polar Bear!” (aimed at Hannes), “Tantra!” (aimed at the Polish beer “Tatra” that was scattered about on the table and “Pizza!” (aimed at the Italians) kept us amused until around 4am, when I gave up and went to bed with a fag-smoke induced headache the size of a beach-ball.

While I tried to sleep, I was reminded of my current location by the constant shouts (see above), phone calls to Iceland and many general beer-fuelled activities, which fortunately involved avoiding the swimming pool. Early days yet…

On Thursday night there were relatively few people – on Friday there will be 80-90, more beer and a casino (With Symfony money no less). Stay tuned.

Day 1 – Community, Casinos and Karaoke

I woke up at about 5 past 10 to a sea of empty Italian beds, which was in plenty of time for the introductory fun and games at 10… hmm… well, anyway, by about 10:15 I had joined the conference and owing to the relaxed atmosphere here it was not embarrassing at all 🙂

Our Dutch host recreated a form of what he described as the Large Hadron Collider, although some may argue that it was just a line of red warning tape. We then proceeded to order ourselves in various categories, like how far we’d travelled (we were down at the end behind the Americans), how many Symfony apps we’d been part of, etc. It was clear that there was a broad mix of experience levels – Fabien at one end (of course) and Symfony virgins at the other.

Hannes was restful during most of the talks, but he made a noticeable shriek when Fabien mentioned that he considered PHP 5.3 to be PHP 6 because it wasn’t backwards compatible with 5.2… He quickly dozed off again though.

I won’t detail the talks here, since they will be well covered elsewhere, but we had an interesting day – often struggling to hear over the pounding rain but generally absorbing all the necessary info. We had a few chats about how the community can get more involved in the Symfony project, and some positive vibes were evident.

I have to mention that the barbecue was top notch, I had no less than 3 different types of veggie burger! They all tasted great along with the ample side salads and dressings – a welcome treat for me.

I also had a chat with Jonathan Wage about one of our future projects and whether Doctrine could “handle” it. I appreciate his time going through it with me and showing me some code examples, and helping me to formulate some options that I could present to the project – top bloke.

The rest of the evening consisted of some nifty casino action with possibly the worlds best croupier. He was letting us “cheat” to an extent which made the game of Blackjack a whole lot more fun – and when the game was over half of the table had watches, phones, wallets, car keys and credit cards down for the final hand. Great stuff. Following that was karaoke, and yes I did sing 2 or 3 numbers after warming up with a song I can’t remember (edit: it was “Pretty Fly for a White Guy”) right now because REM is playing in the background as I write this.

By 2am the barbecues had been lit for “munchie time”… not that I’m implying anything.

Day 2 – Write a ticket, submit a patch, write the documentation

I woke up pretty late today, but I feel refreshed at least! Maybe enough to try the pool later if I can convince anyone to join me… The sun has come out but it’s still a bit chilly all the same.

The weekend has been good, not only for learning, but also for networking – now we’ll be able to put names to faces the next time we are talking to someone in #Symfony or checking the commit logs. It has also increased our understanding of the project and the people involved, which is very important when it comes to trusting it for future developments.

The night ended with “the Italians” making pasta for us, which was a real treat (including my vegetarian option – thanks guys) and lots of wine drinking until the early hours with a couple of the French guys. It was a pretty cold night, so we even sneakily moved our beds inside for a warmer night’s sleep! It’s late in the day on Sunday when I write this, so we’d better get a move on and get ourselves into the City to meet “the Italians” for lunch.

I’ll do some more updates once we’ve left, and also add a few photos at some point.

Useful links

« Previous PageNext Page »