Convert the entity system to PSR-0

also see http://drupal.org/node/1361232#comment-5768718

Yep, we'll need to convert the system to PSR-0. For now I think it's best to just go and add the auto-loaders manually in order to make the tests work. Then once, we've converted the entity api system we can remove those. That way we don't postpone the current entity api patches being worked on.

I would like to postpone this task until (at least) #1361232: Make the taxonomy entities classed objects gets in, because that will be a pain to re-roll due to all the improvements to the entity system in there.

Yay, Taxonomy entity classes is in, can this be activated now?

I think it can be activated, yes, but I'd like to discuss first if it's worth waiting for the remaining entity patches to get in before starting to work on this.

We should at least wait for #1277776: Add generic field/property getters/setters (with optional language support) for entities, as this also touches the entity module directly.
The others don't touch the entity module much/at all, so I think we should be good to go then.

I guess that would make sense, especially because they could probably already be converted and don't conflict with other patches.

I guess we should discuss the naming first anyway.

For entity.module classes I guess things are pretty clear:
Entity -> Drupal\entity\Entity
EntityInterface -> Drupal\entity\EntityInterface
EntityStorageException -> Drupal\entity\StorageException
EntityDatabaseStorageController -> Drupal\entity\DatabaseStorageController
...

However, how's for entity-type providing modules? Let's take the example of comments.

a)

Comment -> Drupal\comment\Comment
CommentStorageController -> Drupal\comment\CommentStorageController

or should we use
b)

Comment -> Drupal\comment\Entity
CommentStorageController -> Drupal\comment\EntityStorageController

When looking at the provided class of comment.module the Drupal\comment\Entity variant is nice as the purpose of the class is clear just with the name.
However, when using the class the naming would become quite quickly ambiguous without aliasing, e.g. think of comment module dealing with comment and node entities. So probably we should go with a).

Yes, a) IMHO. b) would require tons of use X as Y aliases all over the place.

In light of #1507828: [policy, no patch] Revised standards for class naming within namespaces, definitely A. B would be a nightmare.

ah, good!

Ok, here is a first patch converting entity module and taxonomy and comment entities. I've left out the conversion of efq for now, as well as the classes of the old-entity system which will go away anyway.

The patch includes the latest patch from #1277776: Add generic field/property getters/setters (with optional language support) for entities.
It's based upon the entity-psr0 branch from http://drupal.org/sandbox/fago/1497344 - please use Git for further updates of the patch and contact me if you need access.

According to http://drupal.org/node/1353118 I've used full qualified names for documenting hooks - I don't really like it though as it's rather verbose. I'd prefer to always go with imported and so short class names.

#13: d8-entity-psr0.patch queued for re-testing.

Not sure about doing the Entity system, Taxonomy AND Comment all at the same time. We could simplify the patch if we push Taxonomy and Comment to their own issues.

Here's a patch that attempts to just do the base Entity system. It also renames DrupalDefaultEntityController to DefaultEntityController and moves the Controller and Query classes to their own namespace to help clean up the file structure.

Grr, testbot. Passes locally. I'll have to take a look later on. Any assistance appreciated :-) .

Created separate issues for comment and taxonomy:

- #1533020: Convert comment.module entity classes to PSR-0
- #1533022: Convert taxonomy.module entity classes to PSR-0

The File, Node and User conversion patches have already switched to PSR-0.

Re-rolled due to the module per directory patch, added a bunch of missing use statements. This should pass most tests if not all.

use statements for Exceptions are tricky, they are easily overlooked and there isn't much test coverage for them.

#23: 1495024.patch queued for re-testing.

We should also add information about this conversion to the original change notification at http://drupal.org/node/1400186 http://drupal.org/node/1479568 once it is ready.

Needs a re-roll as User.php and Node.php are now in. Need to update those use statements.

Was missing a couple references.

Ok, this one should pass. Was missing a use in the query test and the node revision failures were random and have been fixed.

No idea why my patch has such a big size difference, this is git diff -M, without -M it was about 160kb.

#32: entity-psr0-1404198-32.patch queued for re-testing.

#32: entity-psr0-1404198-32.patch queued for re-testing.

Let's not split them into the namespaces.

Mmm?

#40: 1495024.patch queued for re-testing.

I'd like to have #1550680: Automatically convert PSR-0 namespaces for extensions into upper CamelCase first.

#40: 1495024.patch queued for re-testing.

The controller classes still had the Controller namespace, let's see if this works.

Looks like the module namespace issue is stuck at where it was before, let's try to get stuff in, afaik that one is a search & replace mostly.

PS: Once again, I do not understand how my file size differs that much, I'm either getting ~60kb with -M or 170kb without.

Hm, I think the patch from #40 that I re-rolled is an old one. The one from #38 actually had a comparable file size and the one that I re-rolled seems to be missing the same use in entity_query.test. The other fails I fail to reproduce. Will check tomorrow.

#47: entity-psr0-1404198-47.patch queued for re-testing.

Ok, let's try this again.

Resolved the merge conflicts and also fixed a few wrong namespaces.

Ah, much better. That fail actually makes sense because that's new code that was added since the previous patch. Which means that this patch should be green...

#54: entity-psr0-1495024-54.patch queued for re-testing.

Would help if I'd use the correct namespace.

#58: entity-psr0-1495024-58.patch queued for re-testing.

Re-rolled, the Simpletest conversion broke it, as so many other issues probably...

+++ b/core/modules/entity/entity.api.phpundefined
@@ -22,18 +22,18 @@
+ *     EntityController:attachLoad(), for example 'node_load'.

Shouldn't this be Drupal\entity\EntityController ?

If that should change it should change in a lot of places. Same for EntityControllerInterface

20 days to next Drupal core point release.

Yep, it should. I think I found all of them, please verify :)

I also assume that we can now default to EntityStorageController in hook_entity_info() and merge these two together once file is converted? (separate issue) The only other remaining usage of EntityController is in TestEntityBundleController and as I understand it, entities will have to use entity class and storage controller, right?

+++ b/core/modules/entity/entity.moduleundefined
@@ -221,13 +223,13 @@ function entity_load($entity_type, $id, $reset = FALSE) {
+ * node_entity_info() and the NodeController in node.module as an example.

Isn't the node controller psr-0 alrdy?

+++ b/core/modules/entity/lib/Drupal/entity/Entity.phpundefined
@@ -2,185 +2,12 @@
+use Drupal\entity\EntityInterface;

We are in the entity namespace, shoud we use this line than?

+++ b/core/modules/entity/lib/Drupal/entity/EntityController.phpundefined
@@ -2,62 +2,23 @@
+use Drupal\entity\EntityControllerInterface;

Same question

+++ b/core/modules/entity/lib/Drupal/entity/EntityDatabaseStorageController.phpundefined
@@ -0,0 +1,158 @@
+use Drupal\entity\EntityController;
+use Drupal\entity\EntityStorageControllerInterface;
+use Drupal\entity\EntityInterface;

Same questions

+++ b/core/modules/entity/lib/Drupal/entity/EntityDatabaseStorageController.phpundefined
@@ -0,0 +1,158 @@
+   * Implements EntityStorageControllerInterface::create().

Drupal\entityEntityStorage... ???

+++ b/core/modules/entity/lib/Drupal/entity/EntityInterface.phpundefined
@@ -0,0 +1,185 @@
+   * @see EntityInterface::language()

Probably need long version to

+++ b/core/modules/entity/lib/Drupal/entity/EntityInterface.phpundefined
@@ -0,0 +1,185 @@
+   * @throws EntityStorageException

Same for throws

+++ b/core/modules/entity/lib/Drupal/entity/EntityInterface.phpundefined
@@ -0,0 +1,185 @@
+   * @return EntityInterface

Same for return statements :p

+++ b/core/modules/node/node.api.phpundefined
@@ -529,12 +529,12 @@ function hook_node_insert(Drupal\node\Node $node) {
+ * via classes NodeController and Drupal\entity\EntityController.

Again, I think we have to go over all instances of node Controller again. Probably not in this issue.

I didn't cover everything, lots of return and throws statements still use the short form (certainly in lib\Drupal\entity)

19 days to next Drupal core point release.

Thanks for the review. Ok, ok, I didn't catch them all ;)

- Yep, NodeController should be fixed in a separate issue, especially because they have been renamed to NodeStorageController :)

- Removed unecessary uses.

- Fixed a ton additional references in docblocks.

Thanks! Nice work.

#65: entity-psr0-1495024-65.patch queued for re-testing.

+++ b/core/modules/entity/lib/Drupal/entity/EntityFieldQuery.phpundefined
@@ -1,20 +1,15 @@
+use PagerDefault;

Is PagerDefault a namespace?

+++ b/core/modules/entity/lib/Drupal/entity/EntityInterface.phpundefined
@@ -0,0 +1,185 @@
+   * @throws EntityStorageException

Full namespace path? ;)

+++ b/core/modules/entity/lib/Drupal/entity/EntityInterface.phpundefined
@@ -0,0 +1,185 @@
+   * @throws EntityStorageException

Same

+++ b/core/modules/entity/lib/Drupal/entity/EntityStorageControllerInterface.phpundefined
@@ -0,0 +1,53 @@
+   * @throws EntityStorageException

Same

+++ b/core/modules/entity/lib/Drupal/entity/EntityStorageControllerInterface.phpundefined
@@ -0,0 +1,53 @@
+   * @throws EntityStorageException

Same

18 days to next Drupal core point release.

PagerDefault as a non-namespaced class. Fixed @throws.

>$ grep -rin "@return Entity" *
>$ grep -rin "@param Entity" *
>$ grep -rin "@throws Entity" *

Return no results, good work!

>$ grep -rin "DrupalEntity" *
core/modules/entity/lib/Drupal/entity/EntityControllerInterface.php:24:   * Constructs a new DrupalEntityControllerInterface object.
core/modules/entity/entity.api.php:214:  // DrupalEntityController interface.

2 minor leftovers that need to be fixed.

And fixes these two as well. Also fixed the alternative entity controller example.

Finally I can set this to rtbc :D. (we need a followup for the nodecontroller stuff)

This looks great. I'm assuming the NodeController stuff will go when we remove the EntityController class, which in turn will be after files get converted to classed objects?

Committed/pushed and moving to change notification', this can likely be added to the existing modules one, or we might need to update the entities as classed objects, not sure tbh.

Added entity to http://drupal.org/node/1320394
Thats all we need.

Nope, that's not enough. We need to update the million references to those classes in http://drupal.org/node/1400186 as well.

Change notice is rewritten, only the api docs need a new reference once they are update on api.drupal.org.
Also noticed we missed some docs, here is the followup

Forgot the user entity, will reroll soon :)
I alrdy fixed the taxonomy stuff in the taxonomy to psr-0 conversion.

With user changes

Patch looks good to me, just a bunch of straight forward replacements. RTBC.

Change notices look good as well apart from the mentioned link problem.

Thanks, committed/pushed to 8.x. Looks like the change notice has bee updated again since Berdir's comment, so marking back to fixed.

Restoring title.

jthorson just spotted this in IRC:

function update_prepare_d8_bootstrap() {
  // Allow the database system to work even if the registry has not been
  // created yet.
  include_once DRUPAL_ROOT . '/core/includes/install.inc';
  include_once DRUPAL_ROOT . '/core/modules/entity/entity.controller.inc';
  drupal_bootstrap(DRUPAL_BOOTSTRAP_DATABASE);

Obviously, that line needs to go. Might be the reason for the current random upgrade test failures or not, let's fix this and find out.

Lets see if this is green

EDIT: whoops unneeded

Looks good.

Thanks! Committed/the follow-up to 8.x.