Bug #55309
PackageManager::getPackageOfObject() does not work for proxy objects
| Status: | Resolved | Start date: | 2014-01-24 | |
|---|---|---|---|---|
| Priority: | Should have | Due date: | ||
| Assigned To: | Bastian Waidelich | % Done: | 100% |
|
| Category: | Package | |||
| Target version: | - | |||
| PHP Version: | Complexity: | |||
| Has patch: | No | Affected Flow version: | Flow 2.0.0 |
Description
The method PackageManager::getPackageOfObject() fails to resolve the package of a given object if it's a proxy.
The Flow/Doctrine proxy classes usually reside in a folder underneath Data/Temporary. Comparing those paths with the package root paths leads to invalid results.
Related issues
Associated revisions
[BUGFIX] Make getPackageOfObject() work for proxy objects
The method ``PackageManager::getPackageOfObject()`` failed to
resolve the package of a given object if it was a proxy.
This change fixes this by comparing the namespace of the object
with the package namespaces rather than the file locations.
Background:
The Flow/Doctrine proxy classes usually reside in a folder underneath
``Data/Temporary``. Comparing those paths with the package root paths
led to invalid results.
Change-Id: I49cf6615b15f4414193d4b563dfe11169fcf44b7
Fixes: #55309
Releases: master, 2.0, 2.1
[BUGFIX] Fix PackageManager unit tests
Due to a previous change the PackageManager's unit tests are failing.
This change adjusts the tests to the modified code fixing the tests.
Background:
The method ``getPackageOfObject()`` fixed with
I49cf6615b15f4414193d4b563dfe11169fcf44b7 changes the behavior of the
PackageManager so that it compares namespaces rather than file paths.
Unfortunately this broke unittests that generated dummy files with
invalid PHP namespaces.
Change-Id: I1b6a80d83702c14cc24b4073a02dbf304180955a
Related: #55309
Releases: master
[BUGFIX] Make Doctrine service return correct packages for migrations
With a previous fix ``PackageManager::getPackageOfObject()`` now
compares the namespace of a given object rather than it's location
on the disk.
This broke the behavior of
``Doctrine\Service::getPackageKeyFromMigrationVersion()`` because
doctrine migrations all have the same namespace ``\TYPO3\Flow\..``
With this fix the doctrine service compares the file paths again.
Change-Id: Ibaeea8993644a09c7266ff3725d9912e1b91d8dd
Related: #55309
Releases: master
[BUGFIX] Make getPackageOfObject() work for proxy objects
The method ``PackageManager::getPackageOfObject()`` failed to
resolve the package of a given object if it was a proxy.
This change fixes this by comparing the namespace of the object
with the package namespaces rather than the file locations.
Background:
The Flow/Doctrine proxy classes usually reside in a folder underneath
``Data/Temporary``. Comparing those paths with the package root paths
led to invalid results.
In addition this adjusts the ``getPackageKeyFromMigrationVersion()``
of the ``Doctrine\Service`` not to rely on ``getPackageOfObject()``
as all doctrine migrations reside in the ``TYPO3\Flow`` namespace.
Change-Id: I49cf6615b15f4414193d4b563dfe11169fcf44b7
Fixes: #55309
Releases: master, 2.0, 2.1
[BUGFIX] Make getPackageOfObject() work for proxy objects
The method ``PackageManager::getPackageOfObject()`` failed to
resolve the package of a given object if it was a proxy.
This change fixes this by comparing the namespace of the object
with the package namespaces rather than the file locations.
Background:
The Flow/Doctrine proxy classes usually reside in a folder underneath
``Data/Temporary``. Comparing those paths with the package root paths
led to invalid results.
In addition this adjusts the ``getPackageKeyFromMigrationVersion()``
of the ``Doctrine\Service`` not to rely on ``getPackageOfObject()``
as all doctrine migrations reside in the ``TYPO3\Flow`` namespace.
Change-Id: I49cf6615b15f4414193d4b563dfe11169fcf44b7
Fixes: #55309
Releases: master, 2.0, 2.1
History
#1 Updated by Gerrit Code Review over 1 year ago
- Status changed from Accepted to Under Review
Patch set 1 for branch master of project Packages/TYPO3.Flow has been pushed to the review server.
It is available at https://review.typo3.org/27045
#2 Updated by Gerrit Code Review over 1 year ago
Patch set 2 for branch master of project Packages/TYPO3.Flow has been pushed to the review server.
It is available at https://review.typo3.org/27045
#3 Updated by Bastian Waidelich over 1 year ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 9128a7f1895901f3308bf50986b69da514d61427.
#4 Updated by Gerrit Code Review over 1 year ago
- Status changed from Resolved to Under Review
Patch set 1 for branch 2.0 of project Packages/TYPO3.Flow has been pushed to the review server.
It is available at https://review.typo3.org/27186
#5 Updated by Gerrit Code Review over 1 year ago
Patch set 1 for branch 2.1 of project Packages/TYPO3.Flow has been pushed to the review server.
It is available at https://review.typo3.org/27187
#6 Updated by Adrian Föder over 1 year ago
The current implementation is still not fully well-working; now it only regards the namespace no matter where the class actually lies.
This might lead to unexpected results; already for the Migration classes which have a TYPO3\Flow namespace, but lie in a respective package of course.
TBH I think the initial implementation where the actual class location was determined, was generally the approach which more matched the expectation of the functionality; but of course with the drawback of Proxy classes not working since they live in Data/Temporary; so I think in case of proxy classes it should manually be worked around (to the least by checking instanceof \TYPO3\Flow\Object\Proxy\ProxyInterface and then taking the Namespace into account, because then it's definitely a Flow package class lying in its PSR-0 location etcpp.)
So my suggestion / opinion so far is: building it back to detecting the actual file location; and only if it seems to be a (copied) Proxy, use the NS as package determinator.
#7 Updated by Dominique Feyer over 1 year ago
@afoeder: I agree +1
#8 Updated by Bastian Waidelich over 1 year ago
Good points! I'll try to push a follow up today! But I doubt that we should fall back to NS / path, it should be the one or the other.. I'm sure there is a way to find the original class from the proxy
#9 Updated by Gerrit Code Review over 1 year ago
Patch set 2 for branch 2.1 of project Packages/TYPO3.Flow has been pushed to the review server.
It is available at https://review.typo3.org/27187
#10 Updated by Gerrit Code Review over 1 year ago
Patch set 2 for branch 2.0 of project Packages/TYPO3.Flow has been pushed to the review server.
It is available at https://review.typo3.org/27186
#11 Updated by Bastian Waidelich about 1 year ago
- Status changed from Under Review to Resolved
Applied in changeset a24c68375c3159819c30099706914e022db5d735.