Add support for yii\web\User Components (Identity Property)
#31
Conversation
Added support for IdentityClass Components implemented by yii\web\User. This is acheived by using genric types in yii\web\User (stubs for now). Need to update inline comments (PHPDocs). Need to write tests. Need to upstream the addition of generic type in yii\web\User to Yii.
WalkthroughThis update introduces support for Yii User components with identity classes by extending the ServiceMap to distinguish user components and their identity classes. New stub files for Yii base and web classes/interfaces are added, configuration and test files are updated accordingly, and minor formatting adjustments are made to documentation and configuration files. Changes
Sequence Diagram(s)sequenceDiagram
participant Config as Config Loader
participant ServiceMap
participant AppReflectionExt as ApplicationPropertiesClassReflectionExtension
participant UserComponent as yii\web\User
participant Identity as IdentityClass
Config->>ServiceMap: Load components from config
ServiceMap->>ServiceMap: processComponents()
alt Component is yii\web\User with identityClass
ServiceMap->>ServiceMap: processUserComponent(id, identityClass)
ServiceMap->>UserComponent: Map id to identityClass
else Other component
ServiceMap->>ServiceMap: Register in general components map
end
AppReflectionExt->>ServiceMap: getUserComponentClassById(id)
alt User component found
AppReflectionExt->>AppReflectionExt: Return ComponentPropertyReflection with GenericObjectType<User, IdentityClass>
else Not a user component
AppReflectionExt->>ServiceMap: getComponentClassById(id)
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (5)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 53 59 +6
===========================================
Files 1 1
Lines 79 86 +7
===========================================
+ Hits 79 86 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
wrote basic tests need to update docs
|
yiisoft/yii2#20400 - If this is merged, the added stubs can be removed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/reflection/ApplicationPropertiesClassReflectionExtension.php (1)
153-155:⚠️ Potential issueInconsistency between hasProperty and getProperty for user components.
The
hasPropertymethod doesn't check for user components viagetUserComponentClassById(), butgetPropertycan resolve them. This could lead to scenarios wherehasProperty()returnsfalsebutgetProperty()succeeds.Add user component checking to maintain consistency:
return $classReflection->hasNativeProperty($propertyName) || $this->annotationsProperties->hasProperty($classReflection, $propertyName) - || $this->serviceMap->getComponentClassById($propertyName) !== null; + || $this->serviceMap->getComponentClassById($propertyName) !== null + || $this->serviceMap->getUserComponentClassById($propertyName) !== null;src/reflection/UserPropertiesClassReflectionExtension.php (1)
109-111:⚠️ Potential issueQuestionable logic for identity property checking.
The logic checks
$this->serviceMap->getComponentClassById($propertyName)when$propertyName === 'identity', but "identity" is not a component ID - it's a property on the User class. This condition will likely never be true and seems incorrect.Consider removing this logic or replacing it with appropriate identity property checking:
- if ($propertyName === 'identity' && $this->serviceMap->getComponentClassById($propertyName) !== null) { - return true; - }
🧹 Nitpick comments (4)
stubs/IdentityInterface.stub (1)
5-5: Consider enrichingIdentityInterfacestub with real method signatures
The stub is minimal, but adding thefindIdentity(),getId(),getAuthKey(), andvalidateAuthKey()signatures could improve type checks in IDEs and static analysis.src/reflection/UserPropertiesClassReflectionExtension.php (1)
53-53: Unused ServiceMap dependency.Since the
getPropertymethod no longer uses theServiceMapand the logic inhasPropertyappears incorrect, this dependency may be unnecessary.Consider removing the unused ServiceMap dependency if it's no longer needed after the refactoring.
src/ServiceMap.php (2)
132-135: Add documentation for the new public method.The method implementation is correct, but it lacks PHPDoc documentation that would be consistent with other public methods in the class.
+ /** + * Retrieves the fully qualified identity class name of a Yii User component by its identifier. + * + * Looks up the identity class name registered under the specified user component ID in the internal user component map. + * + * This method enables static analysis tools and IDEs to resolve the actual identity class type of User + * components for accurate generic type inference and autocompletion. + * + * @param string $id User component identifier to look up in the user component map. + * + * @return string|null Fully qualified identity class name, or `null` if not found. + */ public function getUserComponentClassById(string $id): string|null
374-377: Consider adding parameter validation.The method implementation is straightforward, but consider adding validation to ensure the identity class exists and implements the required interface.
private function processUserComponent(string $id, string $identityClass): void { + if (!class_exists($identityClass)) { + throw new RuntimeException(sprintf("Identity class '%s' for user component '%s' does not exist.", $identityClass, $id)); + } + + if (!is_subclass_of($identityClass, \yii\web\IdentityInterface::class)) { + throw new RuntimeException(sprintf("Identity class '%s' for user component '%s' must implement IdentityInterface.", $identityClass, $id)); + } + $this->userComponents[$id] = $identityClass; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
CHANGELOG.md(1 hunks)README.md(3 hunks)extension.neon(1 hunks)phpunit.xml.dist(1 hunks)src/ServiceMap.php(5 hunks)src/reflection/ApplicationPropertiesClassReflectionExtension.php(2 hunks)src/reflection/UserPropertiesClassReflectionExtension.php(1 hunks)stubs/BaseObject.stub(1 hunks)stubs/Component.stub(1 hunks)stubs/Configurable.stub(1 hunks)stubs/IdentityInterface.stub(1 hunks)stubs/User.stub(1 hunks)tests/ServiceMapTest.php(2 hunks)tests/fixture/config.php(2 hunks)tests/stub/MyUser.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/reflection/ApplicationPropertiesClassReflectionExtension.php (2)
src/ServiceMap.php (1)
getUserComponentClassById(132-135)src/reflection/ComponentPropertyReflection.php (1)
ComponentPropertyReflection(37-271)
tests/ServiceMapTest.php (2)
tests/stub/MyUser.php (1)
MyUser(10-36)src/ServiceMap.php (2)
getComponentClassById(127-130)getUserComponentClassById(132-135)
🔇 Additional comments (19)
phpunit.xml.dist (1)
24-24: Trivial whitespace alignment: This change only adjusts indentation of the<directory>element for consistency and has no functional impact.stubs/Configurable.stub (1)
5-5: Approve newConfigurablestub interface
This empty stub correctly defines the interface for static analysis.CHANGELOG.md (1)
10-10: Verify enhancement numbering continuity
EntryEnh #31skips#30. Ensure no enhancement is missing or misnumbered in the changelog.extension.neon (1)
8-13: Approve addition of stub files to configuration
The newstubFileslist correctly registers all custom stub files required for enhanced type reflection.stubs/BaseObject.stub (1)
5-5: Approve newBaseObjectstub class
This stub correctly extends the emptyConfigurableinterface for static mapping purposes.stubs/Component.stub (1)
1-5:✅ Verification successful
LGTM! Verify BaseObject dependency.
The stub correctly defines the basic Component class hierarchy for static analysis.
Verify that BaseObject stub is properly defined and available:
🏁 Script executed:
#!/bin/bash # Description: Verify BaseObject stub exists and check extension configuration # Check if BaseObject stub exists fd -t f "BaseObject" stubs/ # Check extension.neon for stub file configurations cat extension.neon | grep -A 10 -B 5 "stubFiles"Length of output: 435
BaseObject Stub Verified and Properly Configured
- stubs/BaseObject.stub exists.
- extension.neon’s stubFiles list includes stubs/Component.stub and stubs/BaseObject.stub.
No further changes needed—LGTM!
README.md (2)
17-17: LGTM! Clean formatting improvements.Removing trailing spaces after anchor tags improves code quality and consistency.
Also applies to: 21-21, 23-23, 26-26
142-144: LGTM! Proper PHPStan configuration indentation.The
level: 8parameter is now correctly indented within theparametersblock.stubs/User.stub (1)
7-10:✅ Verification successful
LGTM! Excellent generic type implementation.
The template parameter
Tproperly constrained toIdentityInterfaceand the nullable$identityproperty correctly typed withTperfectly implements the feature described in the PR objectives.Verify that IdentityInterface is properly available for the constraint:
🏁 Script executed:
#!/bin/bash # Description: Verify IdentityInterface stub/definition exists # Check for IdentityInterface stub files fd -t f "IdentityInterface" stubs/ # Check if it's defined in extension configuration rg -A 5 -B 5 "IdentityInterface" extension.neonLength of output: 347
IdentityInterface constraint verified, LGTM
- Confirmed
stubs/IdentityInterface.stubexists- Verified it’s listed under
stubFilesinextension.neonNo further changes needed—approving these updates.
tests/ServiceMapTest.php (2)
17-17: LGTM! Proper import for user component testing.Adding the MyUser import is necessary for the new user component test assertions.
115-132:✅ Verification successful
LGTM! Comprehensive user component testing.
The test assertions correctly verify the new functionality:
- User components return
nullfromgetComponentClassById(lines 115-118, 124-127)- User components correctly resolve via
getUserComponentClassByIdtoMyUser::class(lines 119-123, 128-132)- Tests cover both
customUserandcustomInitializedUservariants- Descriptive assertion messages clearly explain expected behavior
This properly validates the separation of user component handling from regular components.
Verify the test configuration includes the expected user components:
🏁 Script executed:
#!/bin/bash # Description: Verify test fixture contains the expected user component configurations # Check test fixture for customUser and customInitializedUser definitions cat tests/fixture/config.php | grep -A 10 -B 2 "customUser\|customInitializedUser" # Verify MyUser stub exists cat tests/stub/MyUser.phpLength of output: 1331
All user component tests and configurations are valid
Verified presence of bothcustomUserandcustomInitializedUserintests/fixture/config.phpand confirmed theMyUserstub intests/stub/MyUser.php. No further changes required.tests/fixture/config.php (1)
5-7: Good test coverage for user components.The configuration provides comprehensive testing for both array-based and direct instantiation approaches for user components with identity classes. This aligns well with the PR objectives to support generic typing for
yii\web\User.Also applies to: 18-22
src/reflection/ApplicationPropertiesClassReflectionExtension.php (1)
102-109: Excellent implementation of generic user component support.The logic correctly uses
getUserComponentClassById()to resolve identity classes and returns a properGenericObjectTyperepresentingyii\web\User<IdentityClass>. This aligns perfectly with the PR objective to enable generic typing for user components.tests/stub/MyUser.php (1)
1-36: Clean and appropriate test stub implementation.The
MyUserclass correctly implementsIdentityInterfacewith simple return values suitable for testing the generic user component functionality. All required methods are properly implemented.src/reflection/UserPropertiesClassReflectionExtension.php (1)
75-77: Verify the delegation to annotations is correct.The change from custom identity property handling to delegating to the annotations extension should be correct if the generic typing is now handled via stubs. However, ensure that the stubs properly define the generic
identityproperty with the correct type.src/ServiceMap.php (4)
14-14: LGTM!The import statement is correctly added and follows the existing import organization.
80-83: LGTM!The new property is correctly typed and follows the existing pattern for component storage.
300-304: LGTM!The logic correctly identifies User class definitions with identityClass configuration and processes them appropriately. The type checks are comprehensive and safe.
290-294:✅ Verification successful
Verify identityClass property access for User instances.
The logic correctly handles User object instances, but direct property access to
$definition->identityClassmay not be safe if the property is private or protected.
🏁 Script executed:
#!/bin/bash # Description: Check the visibility of identityClass property in yii\web\User class # Search for identityClass property declaration in Yii User class ast-grep --pattern $'class User { $$$ $_ $identityClass$$$; $$$ }' # Alternative search for identityClass property rg -A 3 -B 1 "identityClass" --type phpLength of output: 2777
Direct access to identityClass is safe
The
identityClassproperty onyii\web\Useris declared public in the Yii2 framework, so using$definition->identityClassis safe and requires no further changes.
samuelrajan747
changed the title
yii\web\User Components (Identity Property)
Added support for
yii\web\UserComponents.This is achieved by using generic types in
yii\web\User(stubs for now).Need to update inline comments (PHPDocs).
Need to write tests.
Need to upstream the addition of generic type in
yii\web\Userto Yii.Before:
The Problem
As per the yii2 docs.
As of now,
Yii::$app->__user_component_name__->identityafter successful login will return\yii\web\IdentityInterface, irrespective of what is defined in$config['components']['__user_component_name__']['identityClass'].Expected Outcome
Yii::$app->__user_component_name__->identityafter successful login should return instance of class mentioned in$config['components']['__user_component_name__']['identityClass'].After:
Yii::$app->__user_component_name__->identityafter successful login will return whatever class mentioned in$config['components']['__user_component_name__']['identityClass']which should be implementingyii\web\IdentityInterfaceHow it works:
T of yii\web\IdentityInterfaceis added to theyii\web\Userclass in the class level.yii\web\IdentityInterfaceinyii\web\Useris replaced with generic typeT.yii\web\Useris accessed inyii\web\Application, a class property of generic object type with classyii\web\Userand generic typeTis set toidentityClassdefined in$config['components']['__user_component_name__']['identityClass']is returned.identityproperty ofyii\web\Useris accessed, the property reflection is derived from the PHPDoc annotation inyii\web\Useris returned.T of yii\web\IdentityInterfaceofyii\web\Useris set to the$config['components']['__user_component_name__']['identityClass']earlier.Summary by CodeRabbit
Summary by CodeRabbit
BaseObject,Component,Configurable,IdentityInterface, andUser.