testbot playground for #1497374: Switch from Field-based storage to Entity-based storage

If you want to help, please

  1. Pick a failing test from the latest patch
  2. File an issue on https://drupal.org/sandbox/chx/1857558 with the classname as the node title
  3. Fix test
  4. File patch
  5. Rinse and repeat

There are no systematic issues we are aware of, it's the tests themselves that are broken and need to be fixed one by one. Ping chx on IRC for help as necessary.

Files: 
CommentFileSizeAuthor
#132 interdiff_123_124.txt126.06 KBchx
#131 field_storage-testbot-2047777-130.patch603.97 KByched
PASSED: [[SimpleTest]]: [MySQL] 58,178 pass(es).
[ View ]
#128 field_storage-testbot-2047777-127.patch603.91 KByched
FAILED: [[SimpleTest]]: [MySQL] 58,392 pass(es), 32 fail(s), and 0 exception(s).
[ View ]
#126 field_storage-testbot-2047777-126.patch598.55 KByched
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#124 field_storage-testbot-2047777-124.patch598.45 KByched
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch field_storage-testbot-2047777-124.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#123 2047777_122.patch510.95 KBchx
FAILED: [[SimpleTest]]: [MySQL] 58,143 pass(es), 56 fail(s), and 0 exception(s).
[ View ]
#122 2047777_122.patch0 byteschx
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#120 field_storage-testbot-2047777-120.patch604.36 KByched
FAILED: [[SimpleTest]]: [MySQL] 58,048 pass(es), 0 fail(s), and 23 exception(s).
[ View ]
#118 field_storage-testbot-2047777-118.patch601.9 KByched
FAILED: [[SimpleTest]]: [MySQL] 58,121 pass(es), 1 fail(s), and 6 exception(s).
[ View ]
#117 field_storage-testbot-2047777-117.patch601.09 KByched
PASSED: [[SimpleTest]]: [MySQL] 57,976 pass(es).
[ View ]
#116 field_storage-testbot-2047777-116.patch573.76 KByched
PASSED: [[SimpleTest]]: [MySQL] 58,001 pass(es).
[ View ]
#115 field_storage-testbot-2047777-115.patch574.25 KByched
PASSED: [[SimpleTest]]: [MySQL] 58,122 pass(es).
[ View ]
#113 field_storage-testbot-2047777-113.patch573.34 KByched
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#110 field_storage-testbot-2047777-110.patch496.46 KByched
FAILED: [[SimpleTest]]: [MySQL] 57,861 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#110 interdiff.txt16.21 KByched
#109 field_storage-testbot-2047777-109.patch496.15 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 57,760 pass(es).
[ View ]
#109 interdiff.txt2.91 KBswentel
#107 field_storage-testbot-2047777-107.patch496.33 KBswentel
FAILED: [[SimpleTest]]: [MySQL] 57,978 pass(es), 0 fail(s), and 1 exception(s).
[ View ]
#107 interdiff.txt15.88 KBswentel
#105 field_storage-testbot-2047777-105.patch510.97 KBswentel
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch field_storage-testbot-2047777-105.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#105 interdiff.txt15.42 KBswentel
#102 field_storage-testbot-2047777-102.patch492.95 KBswentel
FAILED: [[SimpleTest]]: [MySQL] 56,735 pass(es), 40 fail(s), and 4,739 exception(s).
[ View ]
#102 interdiff.txt30.17 KBswentel
#100 field_storage-testbot-2047777-99.patch486.88 KBswentel
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#97 field_storage-testbot-2047777-97.patch485.94 KBswentel
FAILED: [[SimpleTest]]: [MySQL] 57,441 pass(es), 9 fail(s), and 1,118 exception(s).
[ View ]
#93 field_storage-testbot-2047777-93.patch485.84 KBswentel
FAILED: [[SimpleTest]]: [MySQL] 57,508 pass(es), 34 fail(s), and 10 exception(s).
[ View ]
#93 interdiff-on-interdiff.txt5.02 KBswentel
#89 field_storage-testbot-2047777-89.patch485.08 KByched
PASSED: [[SimpleTest]]: [MySQL] 57,855 pass(es).
[ View ]
#88 field_storage-testbot-2047777-88.patch486.62 KBswentel
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#88 interdiff.txt16.64 KBswentel
#86 field_storage-testbot-2047777-86.patch485.69 KBswentel
FAILED: [[SimpleTest]]: [MySQL] 57,861 pass(es), 32 fail(s), and 7 exception(s).
[ View ]
#84 field_storage-testbot-2047777-84.patch484.4 KBswentel
FAILED: [[SimpleTest]]: [MySQL] 57,380 pass(es), 57 fail(s), and 65 exception(s).
[ View ]
#84 interdiff.txt11.31 KBswentel
#82 field_storage-testbot-2047777-82.patch484.29 KBswentel
FAILED: [[SimpleTest]]: [MySQL] 56,982 pass(es), 330 fail(s), and 97 exception(s).
[ View ]
#80 field_storage-testbot-2047777-80.patch484.88 KByched
FAILED: [[SimpleTest]]: [MySQL] 57,932 pass(es), 0 fail(s), and 2 exception(s).
[ View ]
#78 field_storage-testbot-2047777-78.patch483.37 KBswentel
FAILED: [[SimpleTest]]: [MySQL] 56,478 pass(es), 342 fail(s), and 117 exception(s).
[ View ]
#78 interdiff.txt9.48 KBswentel
#77 field_storage-testbot-2047777-77.patch483.06 KByched
FAILED: [[SimpleTest]]: [MySQL] 57,796 pass(es), 2 fail(s), and 8,515 exception(s).
[ View ]
#77 interdiff.txt23.11 KByched
#72 field_storage-testbot-2047777-72.patch475.21 KByched
FAILED: [[SimpleTest]]: [MySQL] 57,896 pass(es), 3 fail(s), and 2 exception(s).
[ View ]
#72 interdiff.txt27.82 KByched
#71 field_storage-testbot-2047777-71.patch486.67 KByched
PASSED: [[SimpleTest]]: [MySQL] 57,778 pass(es).
[ View ]
#71 interdiff.txt5.45 KByched
#70 field_storage-testbot-2047777-70.patch489.25 KByched
PASSED: [[SimpleTest]]: [MySQL] 57,764 pass(es).
[ View ]
#70 interdiff.txt6.8 KByched
#69 field_storage-testbot-2047777-69.patch486.84 KByched
PASSED: [[SimpleTest]]: [MySQL] 57,938 pass(es).
[ View ]
#69 interdiff.txt2.94 KByched
#66 interdiff.txt9.18 KByched
#62 field_storage-testbot-2047777-62.patch488.94 KByched
PASSED: [[SimpleTest]]: [MySQL] 57,745 pass(es).
[ View ]
#62 interdiff.txt527 bytesyched
#60 field_storage-testbot-2047777-60.patch488.89 KByched
FAILED: [[SimpleTest]]: [MySQL] 57,193 pass(es), 25 fail(s), and 25 exception(s).
[ View ]
#60 interdiff.txt7.08 KByched
#59 2047777_59.patch493.95 KBchx
PASSED: [[SimpleTest]]: [MySQL] 57,360 pass(es).
[ View ]
#57 2047777_57.patch495.36 KBchx
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2047777_57.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#55 2047777_55.patch495.04 KBchx
FAILED: [[SimpleTest]]: [MySQL] 57,700 pass(es), 0 fail(s), and 1 exception(s).
[ View ]
#38 2047777_36.patch471.7 KBchx
FAILED: [[SimpleTest]]: [MySQL] 57,229 pass(es), 18 fail(s), and 8 exception(s).
[ View ]
#34 field_storage-testbot-2047777-34.patch462.15 KByched
FAILED: [[SimpleTest]]: [MySQL] 57,180 pass(es), 29 fail(s), and 39 exception(s).
[ View ]
#32 field_storage-testbot-2047777-32.patch462.14 KByched
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch field_storage-testbot-2047777-32.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#32 interdiff.txt22.83 KByched
#29 field_storage-testbot-2047777-29.patch455.3 KByched
FAILED: [[SimpleTest]]: [MySQL] 56,870 pass(es), 37 fail(s), and 43 exception(s).
[ View ]
#29 interdiff.txt2.65 KByched
#27 field_storage-testbot-2047777-27.patch454.11 KByched
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/file/file.module.
[ View ]
#27 interdiff.txt8.34 KByched
#24 field_storage-testbot-2047777-24.patch450.9 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] 56,264 pass(es), 47 fail(s), and 68 exception(s).
[ View ]
#24 interdiff.txt16.27 KBamateescu
#21 field_storage-testbot-2047777-21.patch440.77 KByched
FAILED: [[SimpleTest]]: [MySQL] 55,680 pass(es), 144 fail(s), and 176 exception(s).
[ View ]
#17 field_storage-testbot-2047777-17.patch446.28 KByched
FAILED: [[SimpleTest]]: [MySQL] 55,965 pass(es), 145 fail(s), and 180 exception(s).
[ View ]
#17 interdiff.txt18.73 KByched
#15 field_storage-testbot-2047777-15.patch437.28 KByched
FAILED: [[SimpleTest]]: [MySQL] 55,583 pass(es), 182 fail(s), and 513 exception(s).
[ View ]
#15 interdiff.txt12.36 KByched
#13 field_storage-testbot-2047777-13.patch428.4 KByched
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch field_storage-testbot-2047777-13.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#13 interdiff.txt1.54 KByched
#11 field_storage-testbot-2047777-11.patch428.39 KByched
FAILED: [[SimpleTest]]: [MySQL] 55,892 pass(es), 122 fail(s), and 181 exception(s).
[ View ]
#11 interdiff.txt2.16 KByched
#9 field_storage-testbot-2047777-9.patch426.79 KByched
FAILED: [[SimpleTest]]: [MySQL] 55,416 pass(es), 135 fail(s), and 171 exception(s).
[ View ]
#9 interdiff.txt2.04 KByched
#6 field_storage-testbot-2047777-5.patch425.62 KByched
FAILED: [[SimpleTest]]: [MySQL] 53,815 pass(es), 202 fail(s), and 242 exception(s).
[ View ]
#4 field_storage-testbot-2047777-4.patch425.62 KByched
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#4 interdiff.txt32.43 KByched
#1 field_storage-testbot-2047777-1.patch411.71 KByched
FAILED: [[SimpleTest]]: [MySQL] 53,245 pass(es), 188 fail(s), and 203 exception(s).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new411.71 KB
FAILED: [[SimpleTest]]: [MySQL] 53,245 pass(es), 188 fail(s), and 203 exception(s).
[ View ]

Current state of the sandbox.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-1.patch, failed testing.

Investigating those numerous "FieldException: Attempt to create an instance of unknown, disabled, or deleted field XXX in Drupal\field\Plugin\Core\Entity\FieldInstance->__construct()" fails.

Status:Needs work» Needs review
StatusFileSize
new32.43 KB
new425.62 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

A couple fixes.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-4.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new425.62 KB
FAILED: [[SimpleTest]]: [MySQL] 53,815 pass(es), 202 fail(s), and 242 exception(s).
[ View ]

Better with a reroll.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-5.patch, failed testing.

Hmm, that's *more* fails :-(.
I can only assume that less early fatals means more asserts get a chance to fail (there is also more passes...)

Status:Needs work» Needs review
StatusFileSize
new2.04 KB
new426.79 KB
FAILED: [[SimpleTest]]: [MySQL] 55,416 pass(es), 135 fail(s), and 171 exception(s).
[ View ]

I can only assume that less early fatals means more asserts get a chance to fail (there is also more passes...)

That was the case. All upgrade tests now failed a bit later, with 2 fails and 4 warnings instead 1 fail and 1 warning in previous patch, that pretty much accounts for the difference...

Attached patch should fix upgrades, and a bunch of other fatals.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-9.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.16 KB
new428.39 KB
FAILED: [[SimpleTest]]: [MySQL] 55,892 pass(es), 122 fail(s), and 181 exception(s).
[ View ]

Other attempt at fixing the upgrade path.

taxonomy_update_8007() accounts for a column being renamed (tid -> target_id)
- It needs to run *before* field_update_8006(), because that one only moves data columns that have the expected column name as per the current field schema (target_id)
It would probably be best if field_update_8006() just moved all the columns it found in the original tables (removing the need to rely on the runtime field schema / columns), but I'm not sure there's a way to do that. @chx ?
- It then needs to hardcode the old table names, and cannot use DatabaseStorageController::_fieldTableName() since that one will return the new table name, which will only exist after field_update_8006().

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-11.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.54 KB
new428.4 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch field_storage-testbot-2047777-13.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Trying something.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-13.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new12.36 KB
new437.28 KB
FAILED: [[SimpleTest]]: [MySQL] 55,583 pass(es), 182 fail(s), and 513 exception(s).
[ View ]

Drats. Re-trying after reroll.
Patch additionally contains a couple more fixes.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-15.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new18.73 KB
new446.28 KB
FAILED: [[SimpleTest]]: [MySQL] 55,965 pass(es), 145 fail(s), and 180 exception(s).
[ View ]

OK, reverted my attempt on the query in field_update_8006() (see interdiff from #13)
Just fixed the method name (->fromQuery() should be from()), and db_select()->fields() needs a table alias.

Plus a couple more fixes here and there.

Identified, but not fixed yet:
- user_update_8012() needs some special care, it tries to write to the old data tables. See fails in UserPictureUpgradePathTest
- Tests having errors like "Attempt to create an instance of unknown, disabled, or deleted field %foo" in FieldInstance->__construct() probably come from tests that try to create two instances on two different entity types, confident that the field exists - works in HEAD, but now they need to create new fields on each entity type... I fixed one in FieldAttachOtherTest
- we do have errors caused by table names getting too long...
I fixed a couple by reducing random strings / field names, but we'll need a more general fix.
- Help welcome on tests for which the qa.d.o only reports "The test did not complete due to a fatal error"

I'm done for today. Help moving this forward is welcome :-)

Regarding the table names: I'm not sure how we're going to escape including an auto truncating mechanism in DSC::_fieldTableName(). That would mean storing and fetching a map in state()...

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-17.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new440.77 KB
FAILED: [[SimpleTest]]: [MySQL] 55,680 pass(es), 144 fail(s), and 176 exception(s).
[ View ]

Reroll after #2045919: Replace all module_implements() deprecated function calls..

Also - I won't be able to handle all the test fails all by myself, need help here :-)

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-21.patch, failed testing.

Issue summary:View changes

Updated issue summary.

I will throw myself into the fray tomorrow.

Status:Needs work» Needs review
StatusFileSize
new16.27 KB
new450.9 KB
FAILED: [[SimpleTest]]: [MySQL] 56,264 pass(es), 47 fail(s), and 68 exception(s).
[ View ]

Rerolled after #2017207: [meta] Complete conversion of users to Entity Field API and fixed a lot of tests. Let's see what's left :)

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-24.patch, failed testing.

w000t !

Status:Needs work» Needs review
StatusFileSize
new8.34 KB
new454.11 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/file/file.module.
[ View ]

Should fix a couple more.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-27.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.65 KB
new455.3 KB
FAILED: [[SimpleTest]]: [MySQL] 56,870 pass(es), 37 fail(s), and 43 exception(s).
[ View ]

Yep, silly me.

In addition, should hopefully fix OptionsWidgetsTest.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-29.patch, failed testing.

Pushed fixes for:
EntityQueryTest
ContentTranslationSyncImageTest
NormalizeTest
NodeAccess*Test
EntityResolverTest
EntityTranslationTest

The remaining fails are in Views and upgrade path.
Will try to look into views before posting an updated patch.

Status:Needs work» Needs review
StatusFileSize
new22.83 KB
new462.14 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch field_storage-testbot-2047777-32.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Should fix at least some views fails.

I'd really welcome help on the upgrade fails, running them locally makes my poor crappy laptop crawl to a corner and die :-p

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-32.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new462.15 KB
FAILED: [[SimpleTest]]: [MySQL] 57,180 pass(es), 29 fail(s), and 39 exception(s).
[ View ]

merged HEAD

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-34.patch, failed testing.

Status:Needs work» Needs review

Here's an attempt at the field upgrade. It introduces a new update field pseud-hook that needs to be documented.

@chx:

No patch :)

StatusFileSize
new471.7 KB
FAILED: [[SimpleTest]]: [MySQL] 57,229 pass(es), 18 fail(s), and 8 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, 2047777_36.patch, failed testing.

FYI - pushed:
- Merged HEAD
- re-fix DenormalizeTest after #1972116: Add REST test coverage for nodes.

Does not deserve a new patch for now...

Pushed fixes for all Views tests. We're getting very close :)

   // Convert user picture to field after the fields and instances are converted
   // to ConfigEntities.
   $dependencies['user'][8011] = array(
-    'field' => 8003,
+    'field' => 8006,
   );

The comment would need to be updated accordingly.
Also, that's not the problem here - user_8011, as currently coded, puts data in the old field tables.
Either it stays this way and needs to run *before* field_8006, or it stays running after but then has to be modified to put data in the new tables.
The latter seems preferable ?

-      'indexes' => $record['data']['indexes'] ?: array(),
+      // Custom indexes are not upgraded.
+      'indexes' => array(),

Why is that ?

Fixed Drupal\file\Tests\FileListingTest and Drupal\system\Tests\System\PasswordHashingTest.

We're down to only two tests:
- Drupal\system\Tests\Upgrade\ModulesDisabledUpgradePathTest
- Drupal\system\Tests\Upgrade\UserPictureUpgradePathTest

@chx, your turn :)

Because... you can't upgrade custom indexes, not easily, not with the column names changing. In case, the table creation simply fails if we copy over the fid index... we can add another hook if you want. Or we can say that people with custom data need to write custom upgrades.

Hm. Then I'm afraid it's more serious than that.

We're not supposed to rely on plugins within updates - #2055605: Disallow use of plugins during update. That means, no asking the "field type" plugin for the schema() of a given field.
#2032369: _update_8003_field_create_field() relies on field schemas never changing and plugins being available, for example, changed _update_8003_field_create_field() calls to have the caller explicitly inject a hardcoded 'schema' for the field to create.

In our case here, this means we're not supposed to call DatabaseStorageController::_fieldSqlSchema($field) in field_update_8006().
And de facto, the current field_update_8006() will fail on fields whose type is unknown.

The ideal flow for field_update_8006() our case would be:
1) Create a new table by just copying the structure of the existing one
Just take what's there and reproduce it as is, no calling runtime code to rebuild the schema we expect (which includes column names mismatches)
2) Copy the relevant values as is (all columns, no renames) from the old table to the new table
3) Drop the 'entity_type' column in the new table
Done - then each field type module can take care of renaming columns and indexes in their own separate updates, that just need to run after field_update_8006().

1) is just "CREATE TABLE new_table LIKE old_table" in MySQL - problem is, of course, I don't know if we have a cross DB / PDO API in Drupal that allows that :-/.

That's the problem: we do not have a way to copy table structures in Drupal or elsewhere.

We could introduce db_copy_table('src', 'dst');.

In MySQL this is CREATE TABLE dst LIKE src

In SQLite we could read the definition of src by SELECT sql FROM sqlite_master WHERE tbl_name='src' and then massage it. We need to deal with CREATE TABLE and CREATE INDEX separately but it is definitely doable.

In PostgreSQL we can do CREATE TABLE dst (LIKE src INCLUDING ALL).

In MSSQL you are SOL.

Oracle has an SQLite-like solution SELECT dbms_metadata.get_ddl( 'TABLE', 'src' ) FROM DUAL;

Hm, also, we currently don't drop the old table at the end ?

We never drop tables during updates -- why would we? Let the users have their data in case we missed something. Dropping tables is easy manually.

[edit: #47 was a crosspost]

Does #46 mean "let's go for it" ?

Also, one issue with our support for db_insert()->from($query) is that it assumes $query selects an explicit list of fields, the code doesn't support "INSERT INTO new SELECT * FROM old WHERE..." (that syntax is supported in MySQL, pgSQL & SQLite, though)

That one is easily worked around, though - first do a "SELECT * FROM old LIMIT 1", then grab keys in the result. Meh.

Yes, #46 means I will add a copy table. Ouchie.

Way cool. Thanks :-))

Re #49,

<?php
    
// Execute the data migration query.
   
$this->connection->insert($new_table)
      ->
from($select)
      ->
execute();
?>

is in core, it's certainly supported.

re #52 - it's not, I tested it :-)
If you look at the existing core examples for ->from($query), $query always uses an explicit list of fields.

I opened #2056363: INSERT INTO table SELECT * FROM ... not supported

Status:Needs work» Needs review
StatusFileSize
new495.04 KB
FAILED: [[SimpleTest]]: [MySQL] 57,700 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

Thanks for that issue, we might be green with it. This contains both the db_copy_table and that INSERT patch for testing purposes.

Of particular note is the passing of the field schema into _fieldSqlSchema to avoid an (impossible) plugin lookup. We already mandate it on a doxygen level for _update_8003_field_create_field and it is indeed specified in both usages of _update_8003_field_create_field so it was easy.

Status:Needs review» Needs work

The last submitted patch, 2047777_55.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new495.36 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2047777_57.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

yay last exception! Very easy fix, just a new test needing a conversion.

Status:Needs review» Needs work

The last submitted patch, 2047777_57.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new493.95 KB
PASSED: [[SimpleTest]]: [MySQL] 57,360 pass(es).
[ View ]

Merge conflict resolved.

StatusFileSize
new7.08 KB
new488.89 KB
FAILED: [[SimpleTest]]: [MySQL] 57,193 pass(es), 25 fail(s), and 25 exception(s).
[ View ]

Yay, green !

Refactored a couple stuff in field_update_8006():
- inlined a couple vars, added a couple comments
- drop the 'entity_type' column in the new migrated tables
Plus:
- moved file_8003 dependency from field_8003 (CMI) to field_8006 (table reorg)
- changed taxonomy_8007 to run after field_8006 - taxonomy_8007 and file_8003 do exactly the same thing, better if they work the same way.

Did not push yet, let's see what the bot says.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-60.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new527 bytes
new488.94 KB
PASSED: [[SimpleTest]]: [MySQL] 57,745 pass(es).
[ View ]

Doh, #60 updated the comment but forgot to actually bump the dependency number for taxonomy_update_8007...

Ok, pushed #60 / #62.

+++ b/core/modules/system/lib/Drupal/system/Tests/Upgrade/FieldUpgradePathTest.phpundefined
@@ -180,10 +179,17 @@ function testFieldUpgradeToConfig() {
     // Check that field values in a pre-existing node are read correctly.
-    $body = node_load(1)->get('body');
-    $this->assertEqual($body->value, 'Some value');
-    $this->assertEqual($body->summary, 'Some summary');
-    $this->assertEqual($body->format, 'filtered_html');
+    $body = db_select(DatabaseStorageController::_fieldTableName($field_entity), 't')
+        ->fields('t')
+        ->condition('entity_id', 1)
+        ->condition('langcode', Language::LANGCODE_NOT_SPECIFIED)
+        ->orderBy('delta')
+        ->condition('deleted', 0)
+        ->execute()
+        ->fetchObject();
+    $this->assertEqual($body->body_value, 'Some value');
+    $this->assertEqual($body->body_summary, 'Some summary');
+    $this->assertEqual($body->body_format, 'filtered_html');

Looks like this change is not needed, FieldUpgradePathTest is still green if I revert it locally.
@chx, is there another reason or can we just revert it ?

Here is my not very technical review. Ignore whatever is irreverent.

+++ b/core/includes/database.incundefined
@@ -680,6 +680,18 @@ function db_rename_table($table, $new_name) {
+ * Copies the structure of a table.
+ *
+ * @param string $source
+ *   The name of the table to be copied.
+ * @param string $destination

@return missing.

+++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityInterface.phpundefined
@@ -75,4 +75,14 @@ public function setStatus($status);
+   * Retrieves the exportable properties of the entity.
+   *
+   * These are the values that get saved into config.

I don't know but this should be on same line till 80 chars limit and no empty line needed.

+++ b/core/lib/Drupal/Core/Entity/DatabaseStorageController.phpundefined
@@ -7,13 +7,15 @@
+use Drupal\field\FieldInfo;
+use Drupal\field\FieldUpdateForbiddenException;
+use Drupal\field\FieldInterface;
+use Drupal\field\FieldInstanceInterface;
use Drupal\Core\Entity\Query\QueryInterface;
use Drupal\Component\Uuid\Uuid;
use Drupal\Component\Utility\NestedArray;
use Drupal\Core\Database\Connection;

order is wrong.

+++ b/core/lib/Drupal/Core/Entity/DatabaseStorageController.phpundefined
@@ -528,42 +536,637 @@ protected function saveRevision(EntityInterface $entity) {
+   * Implements \Drupal\Core\Entity\EntityStorageControllerInterface::getQueryServiceName().

{@inheritdoc}

+++ b/core/lib/Drupal/Core/Entity/DatabaseStorageController.phpundefined
@@ -528,42 +536,637 @@ protected function saveRevision(EntityInterface $entity) {
+   * @param $schema

typehint missing

+++ b/core/lib/Drupal/Core/Entity/DatabaseStorageController.phpundefined
@@ -528,42 +536,637 @@ protected function saveRevision(EntityInterface $entity) {
+   *   The same as a hook_schema() implementation for the data and the

Can we add @see here?

+++ b/core/lib/Drupal/Core/Entity/DatabaseStorageController.phpundefined
@@ -528,42 +536,637 @@ protected function saveRevision(EntityInterface $entity) {
+   * @param $field

typehint missing.

+++ b/core/lib/Drupal/Core/Entity/DatabaseStorageController.phpundefined
@@ -528,42 +536,637 @@ protected function saveRevision(EntityInterface $entity) {
+      return "field_deleted_revision_" . substr(hash('sha256', $field['uuid']), 0, 10);
...
+      return "field_rev_{$field['entity_type']}__{$field['field_name']}";

Why not use rev here as well? I like rev. If it doesn't break api.

+++ b/core/lib/Drupal/Core/Entity/DatabaseStorageControllerNG.phpundefined
@@ -53,8 +54,8 @@ class DatabaseStorageControllerNG extends DatabaseStorageController {
    * Overrides DatabaseStorageController::__construct().

This needs proper doc block.

+++ b/core/lib/Drupal/Core/Entity/EntityFormController.phpundefined
@@ -311,9 +312,9 @@ public function validate(array $form, array &$form_state) {
+      $definitions = \Drupal::entityManager()->getFieldDefinitions($entity_type, $entity->bundle());
+      foreach (field_info_instances($entity_type, $entity->bundle()) as $field_name => $instance) {
@@ -330,7 +331,7 @@ public function validate(array $form, array &$form_state) {
+        $langcode = field_is_translatable($entity_type , field_info_field($entity_type, $field_name)) ? $entity_langcode : Language::LANGCODE_NOT_SPECIFIED;

This should be injected but probably in follow up.

+++ b/core/lib/Drupal/Core/Entity/EntityManager.phpundefined
@@ -104,21 +113,45 @@ class EntityManager extends PluginManagerBase {
+  /**
+   * Add more namespaces to the entity manager.
+   *
+   * This is usually only necessary for uninstall purposes.

This should be on same line

+++ b/core/lib/Drupal/Core/Entity/EntityStorageControllerBase.phpundefined
@@ -234,6 +234,22 @@ public function invokeFieldItemPrepareCache(EntityInterface $entity) {
+   * @param $hook

typehint missing.

+++ b/core/lib/Drupal/Core/Entity/EntityStorageControllerBase.phpundefined
@@ -234,6 +234,22 @@ public function invokeFieldItemPrepareCache(EntityInterface $entity) {
+    module_invoke_all($this->entityType . '_' . $hook, $entity);
...
+    module_invoke_all('entity_' . $hook, $entity, $this->entityType);

We should inject/pass __construct the moduleHandler or at least use Drupal::

+++ b/core/lib/Drupal/Core/Entity/FieldableEntityStorageControllerBase.phpundefined
@@ -0,0 +1,322 @@
+   * @param $entities
...
+   * @param $age

typehint missing.

+++ b/core/lib/Drupal/Core/Entity/FieldableEntityStorageControllerBase.phpundefined
@@ -0,0 +1,322 @@
+          $cid = "field:$this->entityType:$id";

Can we change this to "field:{$this->entityType}:$id"

+++ b/core/lib/Drupal/Core/Entity/FieldableEntityStorageControllerBase.phpundefined
@@ -0,0 +1,322 @@
+   * @return
+   *   Default values (if any) will be added to the $entity parameter for fields

Do we need @return if yes then typehint missing.

+++ b/core/lib/Drupal/Core/Entity/FieldableEntityStorageControllerBase.phpundefined
@@ -0,0 +1,322 @@
+   * Saves field data for an existing entity.
+   *
+   * It should be enough to override doFieldUpdate() instead of this method.
...
+   * Deletes field data for an existing entity. This deletes all revisions of
+   * field data for the entity.
+   *
+   * It should be enough to override doFieldDelete() instead of this method.
...
+   * Delete field data for a single revision of an existing entity. The passed
+   * entity must have a revision ID attribute.
+   *
+   * It should be enough to override doFieldRevisionDelete() instead of this

Should be on same line.

+++ b/core/lib/Drupal/Core/Entity/Query/Sql/Tables.phpundefined
@@ -108,7 +109,7 @@ function addField($field, $type, $langcode) {
+        $field = field_info_field($entity_type, $specifier);
+++ b/core/modules/block/custom_block/custom_block.moduleundefined
@@ -181,13 +181,13 @@ function custom_block_entity_bundle_info() {
+  $field = field_info_field('custom_block', 'block_body');
+++ b/core/modules/comment/comment.moduleundefined
@@ -322,11 +322,11 @@ function comment_node_type_delete($info) {
+  if (!field_read_field('comment', 'comment_body', array('include_inactive' => TRUE))) {
+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentFieldsTest.phpundefined
@@ -45,7 +45,7 @@ function testCommentDefaultFields() {
+    $field = field_info_field('comment', 'comment_body');
@@ -54,7 +54,7 @@ function testCommentDefaultFields() {
+    $field = field_info_field('comment', 'comment_body');
+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentLanguageTest.phpundefined
@@ -71,7 +71,7 @@ function setUp() {
+    $field = field_info_field('comment', 'comment_body');
+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentTranslationUITest.phpundefined
@@ -63,7 +63,7 @@ protected function getTranslatorPermissions() {
+    $field = field_info_field('comment', 'comment_body');
+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentUninstallTest.phpundefined
@@ -42,7 +42,7 @@ protected function setUp() {
+    $field = field_info_field('comment', 'comment_body');
@@ -50,7 +50,7 @@ function testCommentUninstallWithField() {
+    $field = field_info_field('comment', 'comment_body');
@@ -60,12 +60,12 @@ function testCommentUninstallWithField() {
+    $field = field_info_field('comment', 'comment_body');
...
+    $field = field_info_field('comment', 'comment_body');

Can we use Drupal:: and not use deprecated functions anymore?

+++ b/core/modules/content_translation/lib/Drupal/content_translation/Form/TranslatableForm.phpundefined
@@ -74,9 +75,9 @@ public function getCancelPath() {
+  public function buildForm(array $form, array &$form_state, $entity_type = NULL, $field_name = NULL) {

Doc block above this function needs update.

+++ b/core/modules/field/field.crud.incundefined
@@ -45,8 +45,8 @@
+function field_read_field($entity_type, $field_name, $include_additional = array()) {

Doc block needs update.

+++ b/core/modules/field/field.installundefined
@@ -98,6 +94,68 @@ function _update_8003_field_create_instance(array $field_config, array &$instanc
+ *

Extra white space.

+++ b/core/modules/field/field.installundefined
@@ -98,6 +94,68 @@ function _update_8003_field_create_instance(array $field_config, array &$instanc
+ * @param string $entity_type
+ * @param string $bundle
+ * @param int $entity_id
+ * @param int $revision_id
+ * @param int $field_name

Little description will help.

+++ b/core/modules/field/field.views.incundefined
@@ -89,18 +113,23 @@ function field_views_field_label($field_name) {
+function field_views_field_default_views_data(FieldInterface $field, $sql_entity_types) {

Doc block need @param and @return.

+++ b/core/modules/field/field.views.incundefined
@@ -447,8 +488,8 @@ function field_views_field_default_views_data($field) {
+function list_field_views_data($field, $sql_entity_types) {

typehint and doc block.

+++ b/core/modules/field/lib/Drupal/field/FieldInfo.phpundefined
@@ -305,36 +305,38 @@ public function getInstances($entity_type = NULL) {
+   * @param $entity_type
+   *   The entity type.
    * @param $field_name
...
+  public function getField($entity_type, $field_name) {

Typehint.

+++ b/core/modules/field/lib/Drupal/field/FieldInfo.phpundefined
@@ -441,29 +443,31 @@ public function getBundleInstances($entity_type, $bundle) {
+        $loaded_instances = entity_load_multiple('field_instance', array_values($config_ids));

Can we use Drupal::?

I'll try to review remaining patch. Not changing status cuz I don't want anyone to kill me. :D

StatusFileSize
new9.18 KB

Merged & pushed HEAD.

Re @jibran #45: Thanks for the review. Answering below, assuming remarks are numbered.
1. That's for #2056133: Add db_copy_table_schema, that is just included here until it gets in. + I don't think we add @return statements when there is no return value.
2. Nope, 1st line has to be one sentence whenever possible. Then, empty line and additional remarks.
3. I don't think we have an official standard about the order of "use" statements, but why not. And grouping is better indeed. Fixed.
4. Not part of the patch ?
5. 6. 7. Fixed
8. table names are being adjusted in the main issue
9. Not part of the patch
10. Not introduced by the patch + no injection possible in entity controllers for now
11. Same as 2.
12. Fixed
13. This code is just moved around as is, let's keep it that way, there will be issues to remove module_invoke_all()
14. Fixed
15. Why not. Fixed.
16. That comment is moot anyway. Removed.
17. Same as 2. Fixed to split the second sentence to a separate paragraph.
18. Same as 13. This patch is too massive to do that here.
19. 20. 21. Fixed.
22. True, but _update_8000_field_sql_storage_write() is just copied as is from the previous field_sql_storage.install, and I'm still pondering about it. Left as is for now.
23. Fixed
24. Already the case in HEAD, patch is big enough.
25. It's true for most of FieldInfo methods, but fixed this block.
26. Already the case in HEAD, not for this patch.

Pushed the attached interdiff

@yched Thanks for explaining all the points in detail.
Review of remaining patch as promised.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Core/Entity/Field.phpundefined
@@ -452,7 +423,7 @@ public function delete() {
+          $instance_ids[] = "$entity_type.$bundle.$this->name";

Can we use "$entity_type.$bundle.{$this->name}".

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.phpundefined
@@ -7,6 +7,7 @@
+use Drupal\Core\Entity\DatabaseStorageController;
use Drupal\Core\Language\Language;
use Drupal\Core\Entity\EntityInterface;

Order :)

+++ b/core/modules/file/lib/Drupal/file/Tests/FileFieldTestBase.phpundefined
@@ -66,9 +68,10 @@ function getLastFileId() {
+  function createFileField($name, $entity_type, $bundle, $field_settings = array(), $instance_settings = array(), $widget_settings = array()) {
+++ b/core/modules/taxonomy/taxonomy.pages.incundefined
@@ -98,19 +98,21 @@ function taxonomy_term_feed(Term $term) {
+function taxonomy_autocomplete($entity_type, $field_name) {
+++ b/core/modules/taxonomy/taxonomy.views.incundefined
@@ -335,8 +336,8 @@ function taxonomy_views_data_alter(&$data) {
+function taxonomy_field_views_data($field, $sql_entity_types) {
@@ -363,8 +364,8 @@ function taxonomy_field_views_data($field) {
+function taxonomy_field_views_data_views_data_alter(&$data, $field, $sql_entity_types) {

typehint missing.

Thanks.
Fixed, except createFileField(). The whole FileFieldTestBase.php is not correctly hinted, including the sister attachFileField() method; and at this point we're really trying to limit the patch size.

StatusFileSize
new2.94 KB
new486.84 KB
PASSED: [[SimpleTest]]: [MySQL] 57,938 pass(es).
[ View ]

_update_8000_field_sql_storage_write() was still calling field_sql_storage module functions directly, not sure how block upgrade passed so far.
Checking a candidate fix.

StatusFileSize
new6.8 KB
new489.25 KB
PASSED: [[SimpleTest]]: [MySQL] 57,764 pass(es).
[ View ]

Attempt at removing hook_field_storage_pre_[load|insert|update]()

StatusFileSize
new5.45 KB
new486.67 KB
PASSED: [[SimpleTest]]: [MySQL] 57,778 pass(es).
[ View ]

And attempt at handling "table name too long" as per #1497374-146: Switch from Field-based storage to Entity-based storage

StatusFileSize
new27.82 KB
new475.21 KB
FAILED: [[SimpleTest]]: [MySQL] 57,896 pass(es), 3 fail(s), and 2 exception(s).
[ View ]

Alright, then attempt to revert the test changes that were made to avoid "table name too long"

Fantastic. fubhy suggested that we re-enable (!) the modules before uninstalling them so to remove the hacking of EntityManager.

I will explore this during the week.

Edit: nope. berdir points out that enabling might be a problem as dependencies might be missing. We will just leave it in and add a todo to kill the hack when disabled modules are gone.

@chx: OK - you take care of the the todo ?

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-72.patch, failed testing.

Doh, simpletest pefixes are 16 chars, not 14...
Fixed that, and moved the truncate / hash code to a helper method (see patch in the main issue)

Status:Needs work» Needs review
StatusFileSize
new23.11 KB
new483.06 KB
FAILED: [[SimpleTest]]: [MySQL] 57,796 pass(es), 2 fail(s), and 8,515 exception(s).
[ View ]

Trying to fix views integration, as mentioned in #1497374-158: Switch from Field-based storage to Entity-based storage

StatusFileSize
new9.48 KB
new483.37 KB
FAILED: [[SimpleTest]]: [MySQL] 56,478 pass(es), 342 fail(s), and 117 exception(s).
[ View ]

Testing refactoring of FieldInfo::getFieldMap() - $field->getBundles() needed refactoring immediately too.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-78.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new484.88 KB
FAILED: [[SimpleTest]]: [MySQL] 57,932 pass(es), 0 fail(s), and 2 exception(s).
[ View ]

Views intagration - a couple fixes

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-80.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new484.29 KB
FAILED: [[SimpleTest]]: [MySQL] 56,982 pass(es), 330 fail(s), and 97 exception(s).
[ View ]

Some obvious fixes for infomap/getbundles

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-82.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new11.31 KB
new484.4 KB
FAILED: [[SimpleTest]]: [MySQL] 57,380 pass(es), 57 fail(s), and 65 exception(s).
[ View ]

With another obvious fix

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-84.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new485.69 KB
FAILED: [[SimpleTest]]: [MySQL] 57,861 pass(es), 32 fail(s), and 7 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-86.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new16.64 KB
new486.62 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

This should be green. @yched I also had to change code in field.views.inc so I'll push once this one comes back green.

StatusFileSize
new485.08 KB
PASSED: [[SimpleTest]]: [MySQL] 57,855 pass(es).
[ View ]

Hah, this one should hopefully be green too :-)
But no pb, I can reroll on top of your changes ($field['bundles'], right ?)

$field['bundles'] indeed. Bot hasn't even started yet and many waiting in the queue, looks like the merge will probably be tomorrow :)

@swentel - remarks on #88:

+++ b/core/modules/field/field.info.incundefined
@@ -74,18 +74,20 @@ function field_behaviors_widget($op, $instance) {
+   *  An array keyed by entity type. Each value is an array which keys are
+   *  field names and value is an array with two entries:
+   *   - type: The field type.

Space added at the wrong place ;-)

+++ b/core/modules/field/field.views.incundefined
@@ -70,10 +70,10 @@ function field_views_data_alter(&$data) {
+  foreach ($field['bundles'] as $bundle) {
     try {
-      if ($entity_manager->getStorageController($entity_type) instanceof DatabaseStorageController) {
-        $sql_entity_types[] = $entity_type;
+      if ($entity_manager->getStorageController($field->entity_type) instanceof DatabaseStorageController) {
+        $sql_entity_types[] = $field->entity_type;

This was looping on the entity_types, so it looks like the foreach is not needed at all now ?

+++ b/core/modules/field/lib/Drupal/field/FieldInfo.phpundefined
@@ -184,6 +184,7 @@ public function getFieldMap() {
       $this->fieldMap = $map;
       return $map;
+
     }

Needless empty line :-p

+++ b/core/modules/field/lib/Drupal/field/FieldInfo.phpundefined
@@ -192,7 +193,7 @@ public function getFieldMap() {
-        $fields[$field_config['uuid']] = $field_config;
+        $fields[$field_config['entity_type']][$field_config['uuid']] = $field_config;
       }

Shouldn't be needed, it can stay keyed by field UUID ?

+++ b/core/modules/field/lib/Drupal/field/FieldInfo.phpundefined
@@ -441,9 +442,13 @@ public function getBundleInstances($entity_type, $bundle) {
+      foreach ($this->getFieldMap() as $map_entity_type => $fields) {
+        if ($map_entity_type == $entity_type) {
+          foreach ($fields as $field_name => $field_data) {
+            if (in_array($bundle, $field_data['bundles'])) {
+              $config_ids["$entity_type.$field_name"] = "$entity_type.$bundle.$field_name";
+            }
+          }

Then just iterate on $field_map[$entity_type] ?

+++ b/core/modules/field/lib/Drupal/field/Plugin/Core/Entity/Field.phpundefined
@@ -682,13 +680,13 @@ public function hasData() {
-    foreach ($this->getBundles() as $entity_type => $bundle) {
+    foreach ($this->getBundles() as $bundle) {
       // Entity Query throws an exception if there is no base table.
-      $entity_info = \Drupal::entityManager()->getDefinition($entity_type);
+      $entity_info = \Drupal::entityManager()->getDefinition($this->entity_type);
       if (!isset($entity_info['base_table'])) {
         continue;
       }
-      $query = $factory->get($entity_type);
+      $query = $factory->get($this->entity_type);

Similarly, we just use the entity_type, so the loop is not needed anymore ?

+++ b/core/modules/rest/lib/Drupal/rest/LinkManager/RelationLinkManager.phpundefined
@@ -74,14 +74,14 @@ public function getRelations() {
-    foreach (field_info_fields() as $field_info) {
-      foreach ($field_info['bundles'] as $entity_type => $bundles) {
-        foreach ($bundles as $bundle) {
-          $relation_uri = $this->getRelationUri($entity_type, $bundle, $field_info['field_name']);
+    foreach (field_info_fields() as $entity_type => $fields) {
+      foreach ($fields as $field_name => $field_info) {

Whoops. That's correct, but we haven't updated the phpdocs for field_info_fields() & FieldInfo::getFields()
The code to filter out deleted fields in field_info_fields() needs to be updated as well...
My bad :-/, I'll fix that.

(Minor: while we're in there, $field_info could be renamed just $field here.)

Also, keys are not field names here, you need to use $field->name (or $field['field_name'] to keep BC-style)

Regarding the last snippet - I was confused, it's worse than that actually.
field_info_fields() is currently broken, it still returns an array of fields keyed by field names, but that's wrong, there could be several fields with the same name in different entity types.

It needs to return an array keyed by entity type then field name, and then all current uses need to be updated as well :-/
Let's attack this after the current pending changes have been merged though...

So for now, RelationLinkManager::getRelations() should still assume a flat array of fields keyed by names.

StatusFileSize
new5.02 KB
new485.84 KB
FAILED: [[SimpleTest]]: [MySQL] 57,508 pass(es), 34 fail(s), and 10 exception(s).
[ View ]

Right, sometimes you focus so hard on just getting something green .. :)

Update with interdiff on the interdiff. So yeah, RelationLinkManager::getRelations(), that's a weird one, it failed during refactoring, so somehow this magically works, but I'm too tired now how exactly it can work though.

field_info_fields() is currently broken, it still returns an array of fields keyed by field names, but that's wrong, there could be several fields with the same name in different entity types.

Hmm, well fieldInfo->getFields() stores the field by uuid, so I think we're safe no (in regards of uniqueness at least) ?

Also, in getFieldMap(), there's this little piece of comment, especially the second part. We don't check on unknown entity types - not before either - should we add a check here ? There's a check though in getBundleInstances() - but again, a bit too tired right now to recheck. Minor is that 'type' is always overridden, but I guess that's not to critical.

<?php
     
// Filter out instances of inactive fields, and instances on unknown
      // entity types.
     
if (isset($fields[$field_uuid])) {
       
$field = $fields[$field_uuid];
       
$map[$instance_config['entity_type']][$field['name']]['bundles'][] = $instance_config['bundle'];
       
$map[$instance_config['entity_type']][$field['name']]['type'] = $field['type'];
      }
?>

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-93.patch, failed testing.

Status:Needs work» Needs review

Hmm, well fieldInfo->getFields() stores the field by uuid, so I think we're safe no (in regards of uniqueness at least) ?

Right, fieldInfo->getFields() is good, but field_info_fields() has additional code to filter out deleted fields and re-key by field name. It needs to either re-key by entity_type then field_name, or just keep the UUID keys. Either way, phpdoc & consuming code needs to be updated. Well, let's get this green, and we'll see that later.

re: getFieldMap() / "Filter out instances of inactive fields, and instances on unknown entity types"
Hmm, tricky. I opened #2059965: Should FieldInfo::getFieldMap() filter data about unknown entity types. Let's leave it as is for now :-)

The '*' chars are misaligned in the phpdoc for field_info_field_map(), wrong copy paste from getFieldMap() ;-)

And right, 'type' gets overwritten over and over again in getFieldMap(), but it's always the same type, so no need to bother with "set it only the first time around".

StatusFileSize
new485.94 KB
FAILED: [[SimpleTest]]: [MySQL] 57,441 pass(es), 9 fail(s), and 1,118 exception(s).
[ View ]

Should be better, somehow missed my views changes in the last patch. No interdiff. Changed the '*' chars (djeezus man) and refactored RelationLinkManager::getRelations().

Also noticed that hook_field_views_data_views_data_alter() is not documented in any api file. I didn't refactor _field_views_get_sql_entity_types() (just return string instead of array) yet as that would trigger a lot of other changes as well. let's spin that off in a follow up ?

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-97.patch, failed testing.

yes, _field_views_get_sql_entity_types() is removed in the 'views intergration refactor patch'

Status:Needs work» Needs review
StatusFileSize
new486.88 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Right, and that's somehow being annoying at this point. Going for another botcheck first, but some views tests are failing.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-99.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new30.17 KB
new492.95 KB
FAILED: [[SimpleTest]]: [MySQL] 56,735 pass(es), 40 fail(s), and 4,739 exception(s).
[ View ]

Ok, I've merged your patch with mine, let's see - one local test was already ok, so good hopes. interdiff attached.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-102.patch, failed testing.

right, forgot to update the hook implementations ...

Status:Needs work» Needs review
StatusFileSize
new15.42 KB
new510.97 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch field_storage-testbot-2047777-105.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

new patch after yched pushed his views refactoring.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-105.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new15.88 KB
new496.33 KB
FAILED: [[SimpleTest]]: [MySQL] 57,978 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

man ...

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-107.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.91 KB
new496.15 KB
PASSED: [[SimpleTest]]: [MySQL] 57,760 pass(es).
[ View ]

That was a nasty one. I'll push when it comes back green.

StatusFileSize
new16.21 KB
new496.46 KB
FAILED: [[SimpleTest]]: [MySQL] 57,861 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Do not use truncated table names in views_data entries.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-110.patch, failed testing.

yched what else is left, i saw the green in #172 in the parent

Status:Needs work» Needs review
StatusFileSize
new573.34 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Testing reroll this time...

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-113.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new574.25 KB
PASSED: [[SimpleTest]]: [MySQL] 58,122 pass(es).
[ View ]

Missed one change.

StatusFileSize
new573.76 KB
PASSED: [[SimpleTest]]: [MySQL] 58,001 pass(es).
[ View ]

Reroll

StatusFileSize
new601.09 KB
PASSED: [[SimpleTest]]: [MySQL] 57,976 pass(es).
[ View ]

as per the end of #1497374-212: Switch from Field-based storage to Entity-based storage:
- call fieldOp() methods explicitly instead of within invokeHook()
- merge fieldUpdate() & fieldInsert() into fieldSave()

StatusFileSize
new601.9 KB
FAILED: [[SimpleTest]]: [MySQL] 58,121 pass(es), 1 fail(s), and 6 exception(s).
[ View ]

Reroll

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-118.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new604.36 KB
FAILED: [[SimpleTest]]: [MySQL] 58,048 pass(es), 0 fail(s), and 23 exception(s).
[ View ]

Method renames

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-120.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new0 bytes
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

StatusFileSize
new510.95 KB
FAILED: [[SimpleTest]]: [MySQL] 58,143 pass(es), 56 fail(s), and 0 exception(s).
[ View ]

StatusFileSize
new598.45 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch field_storage-testbot-2047777-124.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Method renames (same as #120) + minor adjustements + chx's reroll.

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-124.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new598.55 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Re-reroll...

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-126.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new603.91 KB
FAILED: [[SimpleTest]]: [MySQL] 58,392 pass(es), 32 fail(s), and 0 exception(s).
[ View ]

forum.module changes got reverted in a previous reroll...

Status:Needs review» Needs work

The last submitted patch, field_storage-testbot-2047777-127.patch, failed testing.

Sigh... Pushed yet another reroll, but those fails beat me for today...

Status:Needs work» Needs review
StatusFileSize
new603.97 KB
PASSED: [[SimpleTest]]: [MySQL] 58,178 pass(es).
[ View ]

OK, I think I got it.

StatusFileSize
new126.06 KB

yched, congrats on getting it to green again but I think you deleted field_sql_storage.module which caused the patch to swell to 600kb. But I can't get it back down to 500kb now it seems. I have made an interdiff.

@chx: the patch I post in the helper issue is the "FULL" patch (with old hook docs & field_sql_storage.module deleted and its tests re-added).
The sandbox contains the "REVIEW" patch - except I forgot to push the fix from #131 yesterday night :-/. Done now.

Will post an updated patch in the main issue with a summary of the changes & interdiffs.

Status:Needs review» Fixed

Yay!!!

Status:Fixed» Closed (fixed)

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

Issue summary:View changes

Updated issue summary.