Part of #1976158: Rename entity storage/list/form/render "controllers" to handlers.

EntityStorageControllerInterface => EntityStorageInterface
EntityStorageControllerBase => EntityStorageBase
ConfigStorageController => ConfigEntityStorage
DatabaseStorageController => EntityDatabaseStorage
FieldableEntityStorageControllerInterface => FieldableEntityStorageInterface
FieldableEntityStorageControllerBase => ContentEntityStorageBase
FieldableDatabaseStorageController => ContentEntityDatabaseStorage

Other entity storage controllers => just need to remove "Controller" suffix

EntityManager->getStorageController() => EntityManager->getStorage()

Update entity_get_controller() and other variables and property names

Before:

After:

Files: 
CommentFileSizeAuthor
#53 entity_classes_before.png19.93 KBxjm
#48 2188613-storage-48-interdiff.txt1.66 KBBerdir
#48 2188613-storage-48.patch634.61 KBBerdir
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,890 pass(es).
[ View ]
#46 2188613-storage-46-interdiff.txt7.09 KBBerdir
#46 2188613-storage-46.patch634.62 KBBerdir
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,786 pass(es).
[ View ]
#43 2188613-storage-43-interdiff.txt682 bytesBerdir
#43 2188613-storage-43.patch633.28 KBBerdir
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,683 pass(es), 76 fail(s), and 17 exception(s).
[ View ]
#41 2188613-storage-41-interdiff.txt1.07 KBBerdir
#41 2188613-storage-41.patch633.28 KBBerdir
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]
#39 2188613-storage-39-interdiff.txt6.16 KBBerdir
#39 2188613-storage-39.patch633.13 KBBerdir
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]
#37 2188613-storage-36-interdiff.txt49.09 KBBerdir
#37 2188613-storage-36.patch632.47 KBBerdir
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2188613-storage-36_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#37 after.png17.29 KBBerdir
#37 before.png19.93 KBBerdir
#35 2188613-storage-35.patch632.02 KBBerdir
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,721 pass(es), 3 fail(s), and 2 exception(s).
[ View ]
#35 2188613-storage-35-interdiff.txt4.71 KBBerdir
#33 2188613-storage-33-interdiff.txt817 bytesBerdir
#33 2188613-storage-33.patch634.21 KBBerdir
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2188613-storage-33.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#31 2188613-storage-31-interdiff.txt1.4 KBBerdir
#31 2188613-storage-31.patch631.87 KBBerdir
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,791 pass(es), 10 fail(s), and 46 exception(s).
[ View ]
#29 2188613-storage-29.patch633.4 KBBerdir
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2188613-storage-29_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#26 2188613-storage-26-interdiff.txt31.99 KBBerdir
#26 2188613-storage-26.patch624.66 KBBerdir
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,621 pass(es).
[ View ]
#22 2188613-storage-22-interdiff.txt3.72 KBBerdir

Comments

Xen’s picture

Assigned:Unassigned» Xen

I'll take a stab at this.

Xen’s picture

Some progress, I have a test site up and running again after the great rename. Tomorrow I'll weed out some failing tests.

Xen’s picture

I'm getting some very weird fallout on the tests, that doesn't make much sense. Aggregator fails on feed items never hitting the DB, but entity tests passes. drupal_render and readonly stream wrappers fails.

I have to dig deeper to figure out what goes wrong here..

Xen’s picture

Status:Active» Needs review
StatusFileSize
new881.51 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-controller_rename-2188613-4.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Now that was stupid. twig_debug in settings file killing random tests.

Let's see what the testbot thinks.

Status:Needs review» Needs work

The last submitted patch, 4: drupal-controller_rename-2188613-4.patch, failed testing.

Xen’s picture

Damn, this is virtually impossible to keep up to date with HEAD. Two days, and there's merge conflicts in 26 files.

Berdir’s picture

Well, #2165155: Change $entity_type to $entity_type_id and $entity_info to $entity_type/$entity_types just got in, which changed all the constructor arguments.

That said, looks like your patch does not correctly move the files but deletes and re-adds all content, make sure you have git configured properly, the patch should *not* be almost 1MB :)

You should be able to do the merge by applying on an older version and then rebasing/merging with head, I'd expect that might work for most conflicts.

Xen’s picture

@berdir
Ah, yes that'll explain it.

Which git configuration is that?

Well, the 26 files is in rebasing, mostly the mentioned constructors.

Rebasing as we speak...

Xen’s picture

StatusFileSize
new560.64 KB
FAILED: [[SimpleTest]]: [MySQL] 63,666 pass(es), 69 fail(s), and 10 exception(s).
[ View ]

OK, rerolled.

Xen’s picture

Status:Needs work» Needs review

Woops, and status.

Status:Needs review» Needs work

The last submitted patch, 9: drupal-controller_rename-2188613-9.patch, failed testing.

Xen’s picture

Now that's some odd errors. Token fails?

I'll look into it later.

Xen’s picture

Status:Needs work» Needs review
StatusFileSize
new564.74 KB
PASSED: [[SimpleTest]]: [MySQL] 64,234 pass(es).
[ View ]

Fixed the failing tests and re-rolled.

Berdir’s picture

StatusFileSize
new567.58 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]
new49.41 KB

Very nice work!

Here's a re-roll, tons of conflicts and a few new replacements. Also found a new pattern to replace $this->storageController => $this->storage. somethingStorage or entityStorage might be better, not sure.

Status:Needs review» Needs work

The last submitted patch, 14: drupal-controller_rename-2188613-14.patch, failed testing.

Berdir’s picture

Status:Needs work» Needs review
StatusFileSize
new568.62 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 65,047 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new2.34 KB

Ups.

Status:Needs review» Needs work

The last submitted patch, 16: drupal-controller_rename-2188613-16.patch, failed testing.

andypost’s picture

Status:Needs work» Needs review
StatusFileSize
new879 bytes
new568.62 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 65,053 pass(es).
[ View ]

And another small fix

plach’s picture

Issue summary:View changes
Berdir’s picture

StatusFileSize
new600.29 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,124 pass(es), 93 fail(s), and 63 exception(s).
[ View ]
new101.86 KB

Re-roll and new replacement pattern: "storage controller" => "storage". We however sometimes also refer to config/extension storage as storage controller...

Status:Needs review» Needs work

The last submitted patch, 20: 2188613-storage-20.patch, failed testing.

Berdir’s picture

Status:Needs work» Needs review
StatusFileSize
new602.94 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]
new3.72 KB

Fixed some left-over storageController property usage.

Status:Needs review» Needs work

The last submitted patch, 22: 2188613-storage-22.patch, failed testing.

Sutharsan’s picture

Issue tags:+Needs reroll

Needs reroll. But that may need to wait until #2120871: Rename EntityListController to EntityListBuilder gets committed.

Berdir’s picture

Assigned:Xen» Berdir

Working on a re-roll.

Berdir’s picture

Assigned:Berdir» Unassigned
Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new624.66 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,621 pass(es).
[ View ]
new31.99 KB

Ok, let's try how this goes.

andypost’s picture

@berdir please puthis to field api sandbox, patch looks great but needs +1 to rtbc

Berdir’s picture

Yeah, sorry, I forgot.

We discussed it and we're going to schedule the commit during the next few days, meaning we'll time the re-roll, review and then commit it asap.

Berdir’s picture

StatusFileSize
new633.4 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2188613-storage-29_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Another re-roll. I don't think the sandbox helps because there aren't multiple people working on it, just re-rolls. And I prefer doing those with rebase.

Status:Needs review» Needs work

The last submitted patch, 29: 2188613-storage-29.patch, failed testing.

Berdir’s picture

Status:Needs work» Needs review
StatusFileSize
new631.87 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,791 pass(es), 10 fail(s), and 46 exception(s).
[ View ]
new1.4 KB

You shouldn't be re-rolling this patch against an old HEAD :)

Status:Needs review» Needs work

The last submitted patch, 31: 2188613-storage-31.patch, failed testing.

Berdir’s picture

Status:Needs work» Needs review
StatusFileSize
new634.21 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2188613-storage-33.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new817 bytes

Status:Needs review» Needs work

The last submitted patch, 33: 2188613-storage-33.patch, failed testing.

Berdir’s picture

Status:Needs work» Needs review
StatusFileSize
new4.71 KB
new632.02 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,721 pass(es), 3 fail(s), and 2 exception(s).
[ View ]

And one more...

Status:Needs review» Needs work

The last submitted patch, 35: 2188613-storage-35.patch, failed testing.

Berdir’s picture

Issue summary:View changes
Status:Needs work» Needs review
StatusFileSize
new19.93 KB
new17.29 KB
new632.47 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2188613-storage-36_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new49.09 KB

Ok, here is another update that renames the following classes as discussed (already based on the renames we're already doing):

FieldableEntityStorageInterface => ExtendableEntityStorageInterface
FieldableEntityStorageBase => ContentEntityStorageBase
FieldableDatabaseEntityStorage => ContentEntityDatabaseStorage
DatabaseEntityStorage => EntityDatabaseStorage

I've also made nice pictures of the before after.

Before:

After:

Status:Needs review» Needs work

The last submitted patch, 37: 2188613-storage-36.patch, failed testing.

Berdir’s picture

Status:Needs work» Needs review
StatusFileSize
new633.13 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]
new6.16 KB

And.. another re-roll.

Status:Needs review» Needs work

The last submitted patch, 39: 2188613-storage-39.patch, failed testing.

Berdir’s picture

Status:Needs work» Needs review
StatusFileSize
new633.28 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]
new1.07 KB

One file didn't get renamed. Also found FieldableNullStorage and renamed that too to ContentEntityNullStorage which makes much more sense.

Status:Needs review» Needs work

The last submitted patch, 41: 2188613-storage-41.patch, failed testing.

Berdir’s picture

Status:Needs work» Needs review
StatusFileSize
new633.28 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,683 pass(es), 76 fail(s), and 17 exception(s).
[ View ]
new682 bytes

Too late to work on something like this.

tim.plunkett’s picture

s/Extendable/Extensible? Not sure that extendable is really a word.
Also that interface has no docblock, so it's hard to tell what it's for.

Status:Needs review» Needs work

The last submitted patch, 43: 2188613-storage-43.patch, failed testing.

Berdir’s picture

Status:Needs work» Needs review
StatusFileSize
new634.62 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,786 pass(es).
[ View ]
new7.09 KB

Re-roll, clean-up, fixing tests. Not sure where that text summary change was coming from, reverted, doesn't belong in here.

alexpott’s picture

https://english.stackexchange.com/questions/90426/extensible-vs-extendible

I use the terms as Jim does in the context of Computer Science. In a computer program, if I add subroutines or blocks it is extendable - I can add more blocks. If the program is extensible, the individual blocks may be made more complex by modification, usually adding flexibility.

So this seems Extensible

Berdir’s picture

StatusFileSize
new634.61 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,890 pass(es).
[ View ]
new1.66 KB

Ok, after quite some discussion with @alexpott, we decided to *not* rename the interface. The initial argument for doing it here that it would affect a lot of places, but with the actual implementations renamed to ContentEntity*, which makes a lot of sense and everyone agrees with I think, renaming the interface becomes a trivial task (see interdiff) and can be discussed in #2100343: Remove 'fieldable' key in entity definitions in favour of 'field_ui_base_route'.

alexpott’s picture

Status:Needs review» Reviewed & tested by the community

Great - let's proceed. I've read the patch and checked that all the old names are not appearing in HEAD anywhere.

Berdir’s picture

Issue summary:View changes
catch’s picture

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

  • Commit 2f959b9 on 8.x by catch:
    Issue #2188613 by Berdir, Xen, andypost: Rename EntityStorageController...
xjm’s picture

Issue summary:View changes
StatusFileSize
new19.93 KB

For posterity, here is the "before" with its filename fixed.

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Sutharsan’s picture

For info. I've created this issue to remove some remaining references to Entity Storage Controller (mainly in comments): #2248051: Remove last references to Entity Storage Controller