Skip to content

[Form] The trace of form errors is now displayed in the profiler #12054

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 30, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -473,14 +473,28 @@
{% endif %}
</td>
<td>
{% if error.cause is empty %}
<em>Unknown.</em>
{% elseif error.cause.root is defined %}
<strong>Constraint Violation</strong><br/>
<pre>{{ error.cause.root }}{% if error.cause.path is not empty %}{% if error.cause.path|first != '[' %}.{% endif %}{{ error.cause.path }}{% endif %} = {{ error.cause.value }}</pre>
{% for trace in error.trace %}
{% if not loop.first %}
<br/>Caused by:<br/><br/>
{% endif %}
{% if trace.root is defined %}
<strong>{{ trace.class }}</strong><br/>
<pre>
{{- trace.root -}}
{%- if trace.path is not empty -%}
{%- if trace.path|first != '[' %}.{% endif -%}
{{- trace.path -}}
{%- endif %} = {{ trace.value -}}
</pre>
{% elseif trace.message is defined %}
<strong>{{ trace.class }}</strong><br/>
<pre>{{ trace.message }}</pre>
{% else %}
<pre>{{ trace }}</pre>
{% endif %}
{% else %}
<pre>{{ error.cause }}</pre>
{% endif %}
<em>Unknown.</em>
{% endfor %}
</td>
</tr>
{% endfor %}
Expand Down
9 changes: 9 additions & 0 deletions src/Symfony/Component/Form/Button.php
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,15 @@ public function isSynchronized()
return true;
}

/**
* Unsupported method.
*
* @return null Always returns null
*/
public function getTransformationFailure()
{
}

/**
* Unsupported method.
*
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/Form/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ CHANGELOG
* ObjectChoiceList now compares choices by their value, if a value path is
given
* you can now pass interface names in the "data_class" option
* [BC BREAK] added `FormInterface::getTransformationFailure()`

2.4.0
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,18 +115,39 @@ public function extractSubmittedData(FormInterface $form)
'origin' => is_object($error->getOrigin())
? spl_object_hash($error->getOrigin())
: null,
'trace' => array(),
);

$cause = $error->getCause();

if ($cause instanceof ConstraintViolationInterface) {
$errorData['cause'] = array(
'root' => $this->valueExporter->exportValue($cause->getRoot()),
'path' => $this->valueExporter->exportValue($cause->getPropertyPath()),
'value' => $this->valueExporter->exportValue($cause->getInvalidValue()),
);
} else {
$errorData['cause'] = null !== $cause ? $this->valueExporter->exportValue($cause) : null;
while (null !== $cause) {
if ($cause instanceof ConstraintViolationInterface) {
$errorData['trace'][] = array(
'class' => $this->valueExporter->exportValue(get_class($cause)),
'root' => $this->valueExporter->exportValue($cause->getRoot()),
'path' => $this->valueExporter->exportValue($cause->getPropertyPath()),
'value' => $this->valueExporter->exportValue($cause->getInvalidValue()),
);

$cause = method_exists($cause, 'getCause') ? $cause->getCause() : null;

continue;
}

if ($cause instanceof \Exception) {
$errorData['trace'][] = array(
'class' => $this->valueExporter->exportValue(get_class($cause)),
'message' => $this->valueExporter->exportValue($cause->getMessage()),
);

$cause = $cause->getPrevious();

continue;
}

$errorData['trace'][] = $cause;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should break the loop here (or set $cause to null so that the condition breaks it), otherwise using a different kind of cause triggers an infinite loop.
And you should use the value exporter as well here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed


break;
}

$data['errors'][] = $errorData;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ public function validate($form, Constraint $constraint)
->setParameters(array_replace(array('{{ value }}' => $clientDataAsString), $config->getOption('invalid_message_parameters')))
->setInvalidValue($form->getViewData())
->setCode(Form::ERR_INVALID)
->setCause($form->getTransformationFailure())
->addViolation();
}
}
Expand Down
20 changes: 13 additions & 7 deletions src/Symfony/Component/Form/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,10 @@ class Form implements \IteratorAggregate, FormInterface
private $extraData = array();

/**
* Whether the data in model, normalized and view format is
* synchronized. Data may not be synchronized if transformation errors
* occur.
* @var bool
* Returns the transformation failure generated during submission, if any
* @var TransformationFailedException|null
*/
private $synchronized = true;
private $transformationFailure;

/**
* Whether the form's data has been initialized.
Expand Down Expand Up @@ -634,7 +632,7 @@ public function submit($submittedData, $clearMissing = true)
$viewData = $this->normToView($normData);
}
} catch (TransformationFailedException $e) {
$this->synchronized = false;
$this->transformationFailure = $e;

// If $viewData was not yet set, set it to $submittedData so that
// the erroneous data is accessible on the form.
Expand Down Expand Up @@ -711,7 +709,15 @@ public function isBound()
*/
public function isSynchronized()
{
return $this->synchronized;
return null === $this->transformationFailure;
}

/**
* {@inheritdoc}
*/
public function getTransformationFailure()
{
return $this->transformationFailure;
}

/**
Expand Down
12 changes: 12 additions & 0 deletions src/Symfony/Component/Form/FormInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\Form;

use Symfony\Component\Form\Exception\TransformationFailedException;

/**
* A form group bundling multiple forms in a hierarchical structure.
*
Expand Down Expand Up @@ -230,10 +232,20 @@ public function isEmpty();
/**
* Returns whether the data in the different formats is synchronized.
*
* If the data is not synchronized, you can get the transformation failure
* by calling {@link getTransformationFailure()}.
*
* @return bool
*/
public function isSynchronized();

/**
* Returns the data transformation failure, if any.
*
* @return TransformationFailedException|null The transformation failure
*/
public function getTransformationFailure();

/**
* Initializes the form tree.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ public function testExtractSubmittedDataStoresErrors()
'norm' => "'Foobar'",
),
'errors' => array(
array('message' => 'Invalid!', 'origin' => null, 'cause' => null),
array('message' => 'Invalid!', 'origin' => null, 'trace' => array()),
),
'synchronized' => 'true',
), $this->dataExtractor->extractSubmittedData($form));
Expand All @@ -340,7 +340,7 @@ public function testExtractSubmittedDataStoresErrorOrigin()
'norm' => "'Foobar'",
),
'errors' => array(
array('message' => 'Invalid!', 'origin' => spl_object_hash($form), 'cause' => null),
array('message' => 'Invalid!', 'origin' => spl_object_hash($form), 'trace' => array()),
),
'synchronized' => 'true',
), $this->dataExtractor->extractSubmittedData($form));
Expand All @@ -360,7 +360,12 @@ public function testExtractSubmittedDataStoresErrorCause()
'norm' => "'Foobar'",
),
'errors' => array(
array('message' => 'Invalid!', 'origin' => null, 'cause' => 'object(Exception)'),
array('message' => 'Invalid!', 'origin' => null, 'trace' => array(
array(
'class' => "'Exception'",
'message' => "''",
),
)),
),
'synchronized' => 'true',
), $this->dataExtractor->extractSubmittedData($form));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,11 +225,14 @@ function () { throw new TransformationFailedException(); }

$this->validator->validate($form, new Form());

$is2Dot4Api = Validation::API_VERSION_2_4 === $this->getApiVersion();

$this->buildViolation('invalid_message_key')
->setParameter('{{ value }}', 'foo')
->setParameter('{{ foo }}', 'bar')
->setInvalidValue('foo')
->setCode(Form::ERR_INVALID)
->setCause($is2Dot4Api ? null : $form->getTransformationFailure())
->assertRaised();
}

Expand Down Expand Up @@ -259,11 +262,14 @@ function () { throw new TransformationFailedException(); }

$this->validator->validate($form, new Form());

$is2Dot4Api = Validation::API_VERSION_2_4 === $this->getApiVersion();

$this->buildViolation('invalid_message_key')
->setParameter('{{ value }}', 'foo')
->setParameter('{{ foo }}', 'bar')
->setInvalidValue('foo')
->setCode(Form::ERR_INVALID)
->setCause($is2Dot4Api ? null : $form->getTransformationFailure())
->assertRaised();
}

Expand Down Expand Up @@ -293,10 +299,13 @@ function () { throw new TransformationFailedException(); }

$this->validator->validate($form, new Form());

$is2Dot4Api = Validation::API_VERSION_2_4 === $this->getApiVersion();

$this->buildViolation('invalid_message_key')
->setParameter('{{ value }}', 'foo')
->setInvalidValue('foo')
->setCode(Form::ERR_INVALID)
->setCause($is2Dot4Api ? null : $form->getTransformationFailure())
->assertRaised();
}

Expand Down