feat: add EntityInterface
The main CodeIgniter entity couldn't be replaced with a custom one because the database drivers checked to see if entities were extensions of the entity itself and not an interface. Creating an interface allows for a completely new entity to implement the interface instead of having to extend the Entity itself.
Checklist:
- [x] Securely signed commits
- [ ] Component(s) with PHPDoc blocks, only if necessary or adds value
- [ ] Unit testing, with >80% coverage
- [ ] User guide updated
- [x] Conforms to style guide
I'm all for this. I am not sure if all of the methods in the interface need to be in the interface, though, and will have to look over that later.
They probably don't from a methods utilized within the framework itself standpoint, we'd need to identify which ones are being used though to include. This is just all the public methods which seemed like the safest place to start.
In this case, the interface needs only the injectRawData() method.
If you need other methods, it is better to add other interfaces.
See https://en.wikipedia.org/wiki/Interface_segregation_principle
Is this a bug fix? If not, please go to 4.5 branch.
In this case, the interface needs only the
injectRawData()method. If you need other methods, it is better to add other interfaces.
I think given that this is to align with how you'd build a replacement entity, you'd at minimum want to require the implementation of those methods used in the model such as toRawArray()
Here's my list of methods we would want in the interface:
-
fill- it's too much a part of how I use an entity to imagine it not being there. Especially dealing with data from forms. -
toArrayandtoRawArray -
hasChanged -
injectRawDatasince that's how the database injects data.
To me, those 5 methods are what defines the core of the functionality if you were to create your own.
Indeed, injectRawData() and toRawArray() can be considered a set.
But there is an opinion that hasChanged() should be removed. It is not a must.
It is better to add as another interface, if we create interface for it.
I would be in favor of adding bare minimum methods to the interface, which will ensure that everything will work properly with our model. While we may be used to more methods, I feel the custom Entity implementation should not impose anything beyond what is required.
That being said I agree with @kenjis - we need only injectRawData() and toRawArray() since no other methods are required elsewhere.
This option would also require a little mention in the user guide.
If thinking of it purely as a "what does the framework need" to work with an Entity than I agree with both of you. I was thinking of it more from a user's perspective of what's the bare minimum functionality I would need to be able to reasonably swap out usage of Entity types.
With the point of this interface being to allow for the safe creation of a custom entity, and assuming that the only items required by the framework are injectRawData() and toRawArray() it probably makes sense to just include those. The others are all convenience and really the point of being able to create your own entity is that you might want to handle data differently or create a much more minimal entity and therefore the others probably shouldn't be required.
Even hasChanged() while likely something that would be pretty common is not required by the other 2 toRawArray() alreaady takes care of the change state through arguments.
@colethorsen - out of curiosity, why are you implementing your own Entity class? Are you missing some functionality or just have a custom use case?
Thank you for updating.
I think this is an enhancement, not a bug fix.
So please change the base branch to 4.5.
https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#if-you-sent-to-the-wrong-branch
@colethorsen - out of curiosity, why are you implementing your own Entity class? Are you missing some functionality or just have a custom use case?
Cole is one of our resident NoSQL devs. In my work with Firebase I definitely had to hack up Entity to get things to work, and eventually ditched it altogether - so my guess is that he's doing the same.
Please fix the code, and re-run composer cs-fix.
1 file with changes
===================
1) system/Entity/EntityInterface.php:0
---------- begin diff ----------
@@ @@
<?php
+declare(strict_types=1);
+
/**
* This file is part of CodeIgniter 4 framework.
*
@@ @@
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/
-
namespace CodeIgniter\Entity;
/**
----------- end diff -----------
Thank you. Can you add a short description to the changelog? https://github.com/codeigniter4/CodeIgniter4/blob/4.5/user_guide_src/source/changelogs/v4.5.0.rst#enhancements
@codeigniter4/core-team Is there no need to add test code?
:wave: Hi, @colethorsen!
We detected conflicts in your PR against the base branch :speak_no_evil:
You may want to sync :arrows_counterclockwise: your branch with upstream!
Ref: Syncing Your Branch
BaseResult still depends on Entity.
Replace it with the interface.
https://github.com/codeigniter4/CodeIgniter4/blob/b17255a43fbe94dd6fcaeebc93303e04eae32bc6/system/Database/BaseResult.php#L16
https://github.com/codeigniter4/CodeIgniter4/blob/b17255a43fbe94dd6fcaeebc93303e04eae32bc6/system/Database/BaseResult.php#L162
https://github.com/codeigniter4/CodeIgniter4/blob/b17255a43fbe94dd6fcaeebc93303e04eae32bc6/system/Database/BaseResult.php#L243
https://github.com/codeigniter4/CodeIgniter4/blob/b17255a43fbe94dd6fcaeebc93303e04eae32bc6/system/Database/BaseResult.php#L542
Fix the PHPStan errors. Remove the following Ignored error pattern in phpstan-baseline.php.
Error: Ignored error pattern #^Method CodeIgniter\\Entity\\Entity\:\:injectRawData\(\) has parameter \$data with no value type specified in iterable type array\.$# in path /home/runner/work/CodeIgniter4/CodeIgniter4/system/Entity/Entity.php was not matched in reported errors.
Error: Ignored error pattern #^Method CodeIgniter\\Entity\\Entity\:\:toRawArray\(\) return type has no value type specified in iterable type array\.$# in path /home/runner/work/CodeIgniter4/CodeIgniter4/system/Entity/Entity.php was not matched in reported errors.
------ -----------------------------------------------------------------------
Line system/Entity/Entity.php
------ -----------------------------------------------------------------------
Ignored error pattern #^Method
CodeIgniter\\Entity\\Entity\:\:injectRawData\(\) has parameter \$data
with no value type specified in iterable type array\.$# in path
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Entity/Entity.php
was not matched in reported errors.
Ignored error pattern #^Method
CodeIgniter\\Entity\\Entity\:\:toRawArray\(\) return type has no
value type specified in iterable type array\.$# in path
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Entity/Entity.php
was not matched in reported errors.
------ -----------------------------------------------------------------------
Error: [ERROR] Found 2 errors
https://github.com/codeigniter4/CodeIgniter4/actions/runs/8423534292/job/23065390399?pr=8325
:wave: Hi, @colethorsen!
We detected conflicts in your PR against the base branch :speak_no_evil:
You may want to sync :arrows_counterclockwise: your branch with upstream!
Ref: Syncing Your Branch
Resolve conflicts, please.
We will release v4.5.0 soon. If you want this to be included, fix the issue.