Prevent empty values to be attached to the controller query

I'm not comfortable with this failing silently. Let's fix the problem, but make it error out if an empty value is passed.

Let's improve the error message a bit by adding the name of the field that the empty condition is trying to be attached to. It'll make debugging a lot easier. With that change, it's RTBC.

In the table versioncontrol_repositories you have a field called update_method - which will be either 0 or 1, as per these:

define('VERSIONCONTROL_UPDATE_LOG_PARSE_ON_CRON', 0);
define('VERSIONCONTROL_UPDATE_INDEPENDENT_EXTERNAL_SCRIPTS', 1);

Setting the update method to parse on cron, value 0, leads to the check introduced in the above patch failing and throwing the exception:

if (!empty($value)) {
  $this->attachCondition($query, $field, $value);
}
else {
  throw new Exception(t('Attempted to attach an empty condition on field @field with the controller @controller.', array('@field' => $field, '@controller' => get_class($this))), E_ERROR);
}

Which in turn leads to an uncaught exception and breaks the entire cron.php run completely. I'm assuming that is not intentional?

A caveat with with empty() is that is considers the following to be "empty":

"" (an empty string)
0 (0 as an integer)
0.0 (0 as a float)
"0" (0 as a string)
NULL
FALSE
array() (an empty array)
var $var; (a variable declared, but without a value in a class)

It's therefore not suitable for checking if a mixed variable is "empty", where 0 is considered an acceptable "non-empty" value.

ahh, good call, that should be an isset().

this'll be a problem with versioncontrol_git's cron hook...crap. i need to roll a new release before we deploy this to d.o.

k, pushed a fix, and tagging a new release. thanks for the report, hoesi.