Created on 24 February 2024, 4 months ago

Problem/Motivation

As noticed by some folks here #2708529: How to add data programatically? β†’ if you just set $entity->geofield->value the additional field values do not get computed. This is because GeofieldItem::setValue() is not called.
When loading the entity anew they do get computed (unnecessarily even if they were already computed and stored) when you get a property like $entity->geofield->lat because it that causes a ::setValue() on first read, and the setValue always generates the computed values.
That this (re-)calculation happens is also relied on by the test GeofieldItemTest::testCrudAndUpdate() which looks like it should be checking that the value is calculated before storage.

Steps to reproduce

One option. Create an entity; only set the geofield by $entity->geofield->value and save. Look in the database.

Better option. I'll push a branch with the GeofieldItemTest::testCrudAndUpdate() test updated to look at the value that was actually saved (and loaded).

Proposed resolution

Ideally the calculation should be done as little as possible. Assuming the data is desirable, it's the reason to store the calculated values in the database. So code to: only, but always, calculate the values when the value property is set, or changed, with the additional properties missing. When loading from the db all the values should be set, and it not need recalculating. When creating a new value for the field it should be calculated, although it might be nice if the calculated values are supplied it's not recalculated. Also when changing the value property.

Remaining tasks

Add failing test.
Fix.

API changes

Will work as expected when using the 'shorthand' entity api access to set the value.

πŸ› Bug report
Status

Active

Version

1.0

Component

Code

Created by

πŸ‡³πŸ‡±Netherlands ekes

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @ekes
  • πŸ‡³πŸ‡±Netherlands ekes

    Pushed an update to the test in the branch which will fail because all the computed values are loaded from the db as NULL.

    It uses a reflection class to get the field values from the entity as they came from the EntityLoad without doing any field loading.

        // Before accessing the entity field and incidentally trigger a calculation.
        $relection = new \ReflectionClass($entity);
        $property = $relection->getProperty('values');
        $property->setAccessible(TRUE);
        // Retrieve the values as they came from storage.
        $values = $property->getValue($entity);
    

    The first test added passes as the $entity->geofield_field->value does set a wkt value to the value property and this is stored.

        // Check stored set value.
        $this->assertEquals($value, $values['geofield_field']['x-default'][0]['value']);
    

    But the following tests all fail because while the property values have been loaded from the database, they are all NULL.

        // Check stored computed values.
        $geom = \Drupal::service('geofield.geophp')->load($value);
        $centroid = $geom->getCentroid();
        $bounding = $geom->getBBox();
        $computed = [];
    
        $computed['geo_type'] = $geom->geometryType();
        $computed['lon'] = $centroid->getX();
        $computed['lat'] = $centroid->getY();
        $computed['left'] = $bounding['minx'];
        $computed['top'] = $bounding['maxy'];
        $computed['right'] = $bounding['maxx'];
        $computed['bottom'] = $bounding['miny'];
        $computed['geohash'] = $geom->out('geohash');
    
        foreach ($computed as $index => $computed_value) {
          $this->assertEquals($computed_value, $values['geofield_field']['x-default'][0][$index]);
        }
    
Production build 0.69.0 2024