Bug #7182
Throw an exception if class for annotated dataType is missing
| Status: | Resolved | Start date: | 2010-08-18 | |
|---|---|---|---|---|
| Priority: | Should have | Due date: | ||
| Assigned To: | - | % Done: | 0% |
|
| Category: | MVC | |||
| Target version: | - | Estimated time: | 1.00 hour | |
| PHP Version: | Complexity: | |||
| Has patch: | No | Affected Flow version: | FLOW3 1.0.0 |
Description
Hey,
if you have a typo in the annotation of an argument of your method, FLOW3 throws some kind of confusing exception. Let's assume we want to hand over an object and want to annotate the var F3\SomePackage\Model\Domain\SomeModel but we misspell and actually type F3\SomePackage\Model\Typo\SomeModel. What FLOW3 throws is:
#1: Catchable Fatal Error: Argument 1 passed to F3\SomePackage\Controller\SomeController::someAction() must be an instance of F3\SomePackage\Model\Typo\SomeModel, array given in /some/fancy/path/flow3/Packages/Application/SomePackage/Classes/Controller/SomeController.php line someHighNumber
That's confusing, because it's no problem that the argument is an array but that the corresponding class is not loaded and the DataMapper is not able to map the argument to an object.
This is generated in F3\FLOW3\MVC\Controller\Argument.php:435 where the value of an argument is simply returned if the corresponding class does not exist. This avoides mapping the argument to an object and we end up with the wrong type but not revealing this to the user.
I think we should catch this !class_exists() and check, if the annotated datatype is user-/framework-defined (matching F3\-schema). If it is, we throw an exception that we were not able to map because of the missing file. If it is an primitive type (incl. validation-related types) it's ok to just return the value.
flow3_7182_patch.diff
- Patch 7182 v1
(2.6 kB)
History
#1 Updated by Felix Oertel over 5 years ago
- File flow3_7182_patch.diff added
I filed a bug report for this before in #7076 for extbase. But I agree with Jochen that we should solve this in FLOW3 first and then port it back.
Patch attached.
#2 Updated by Christopher Hlubek over 5 years ago
- Assigned To set to Robert Lemke
@Robert: We talked about this issue in Lübeck already. It's hard to track this error and the framework should provide a more meaningful error here.
I walked through the code with Felix today and we thought about when to throw an exception for a missing class. It's not enough just to skip primitive types since the data type in the argument can be a virtual type (e.g. "Text"). But I'm not sure if it's okay to match every class in "F3" for a missing class exception.
#3 Updated by Robert Lemke about 5 years ago
- Status changed from New to Needs Feedback
- Assigned To changed from Robert Lemke to Christopher Hlubek
- Target version set to 1.0 alpha 11
Christopher Hlubek wrote:
@Robert: We talked about this issue in Lübeck already. It's hard to track this error and the framework should provide a more meaningful error here.
I walked through the code with Felix today and we thought about when to throw an exception for a missing class. It's not enough just to skip primitive types since the data type in the argument can be a virtual type (e.g. "Text"). But I'm not sure if it's okay to match every class in "F3" for a missing class exception.
What do you mean with "virtual type" - a non-qualified class name?
It's really a bit tricky because we basically have two problems here:
- the mapper should fail earlier and produce a proper error message that an object of that name (sic!) is not registered with FLOW3
- the fatal error when calling the method with wrong arguments should be caught and transformed in to a more meaningful exception
#4 Updated by Karsten Dambekalns almost 5 years ago
Hi,
I'm puzzled by this:
Christopher Hlubek wrote:
It's not enough just to skip primitive types since the data type in the argument can be a virtual type (e.g. "Text").
FLOW3 evaluates the @param annotation in this case, and there you must use valid types, i.e. primitives of fully qualified class names.
No?
#5 Updated by Karsten Dambekalns almost 5 years ago
- Status changed from Needs Feedback to Accepted
- Assigned To changed from Christopher Hlubek to Karsten Dambekalns
- Start date changed from 2010-04-08 to 2010-08-18
- Estimated time set to 1.00
#6 Updated by Karsten Dambekalns almost 5 years ago
- Assigned To deleted (
Karsten Dambekalns) - Target version deleted (
1.0 alpha 11)
Karsten Dambekalns wrote:
FLOW3 evaluates the @param annotation in this case, and there you must use valid types, i.e. primitives of fully qualified class names.
Wrong. Dang, the $dataType can indeed be something like "Text". So this needs more thought than sensible one day before a release...
Observation: If you check for no backslash in the type and return the value in those cases, you get a step further. For TYPO3 Phoenix this doesn't help, though, as you get an exception on a type that is an interface, not a class. If you then add interface_exists() to class_exists(), it complains that the value was "a simple type" - but it was a Page proxy already.
#7 Updated by Karsten Dambekalns almost 4 years ago
- Affected Flow version set to FLOW3 1.0.0
#8 Updated by Christian Müller over 3 years ago
- Status changed from Accepted to Resolved
- Has patch set to No
The "new" PropertyMapper catches this error already and gives a resonable message.