Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH PHP 8.2 support #10614

Merged
merged 1 commit into from
Dec 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"composer/installers": "^2.2",
"guzzlehttp/guzzle": "^7.5.0",
"guzzlehttp/psr7": "^2.4.0",
"embed/embed": "^4.4.4",
"embed/embed": "^4.4.7",
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
"league/csv": "^9.8.0",
"m1/env": "^2.2.0",
"monolog/monolog": "^3.2.0",
Expand Down
2 changes: 1 addition & 1 deletion src/Control/HTTPRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ public function match($pattern, $shiftOnSuccess = false)
$shiftCount = sizeof($patternParts ?? []);
$remaining = count($this->dirParts ?? []) - $i;
for ($j = 1; $j <= $remaining; $j++) {
$arguments["$${j}"] = $this->dirParts[$j + $i - 1];
$arguments['$' . $j] = $this->dirParts[$j + $i - 1];
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
}
$patternParts = array_merge($patternParts, array_keys($arguments ?? []));
break;
Expand Down
2 changes: 2 additions & 0 deletions src/Dev/Constraint/SSListContains.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ class SSListContains extends Constraint implements TestOnly
*/
protected $matches = [];

protected SSListExporter $exporter;
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved

/**
* Check if the list has left over items that don't match
*
Expand Down
2 changes: 2 additions & 0 deletions src/Dev/Constraint/SSListContainsOnlyMatchingItems.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ class SSListContainsOnlyMatchingItems extends Constraint implements TestOnly
*/
private $match;

protected SSListExporter $exporter;
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved

/**
* @var ViewableDataContains
*/
Expand Down
13 changes: 13 additions & 0 deletions src/Dev/FixtureBlueprint.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ class FixtureBlueprint
*/
protected $class;

private FixtureFactory $factory;

/**
* @var array
*/
Expand Down Expand Up @@ -70,6 +72,17 @@ public function __construct($name, $class = null, $defaults = [])
$this->defaults = $defaults;
}

public function getFactory(): FixtureFactory
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
{
return $this->factory;
}

public function setFactory(FixtureFactory $factory): static
{
$this->factory = $factory;
return $this;
}

/**
* @param string $identifier Unique identifier for this fixture type
* @param array $data Map of property names to their values.
Expand Down
2 changes: 2 additions & 0 deletions src/Dev/SapphireTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ abstract class SapphireTest extends TestCase implements TestOnly
*/
protected static $tempDB = null;

protected FixtureFactory|bool $fixtureFactory;
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved

/**
* @return TempDatabase
*/
Expand Down
2 changes: 2 additions & 0 deletions src/ORM/Connect/MySQLDatabase.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ class MySQLDatabase extends Database implements TransactionManager
*/
private $transactionManager = null;

private int $transactionNesting = 0;
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved

/**
* Default collation
*
Expand Down
1 change: 0 additions & 1 deletion src/ORM/Connect/MySQLStatement.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ public function __construct($statement, $metadata)
public function __destruct()
{
$this->statement->close();
$this->currentRecord = false;
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
77 changes: 42 additions & 35 deletions src/ORM/DataObject.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Exception;
use InvalidArgumentException;
use LogicException;
use SilverStripe\Assets\Storage\DBFile;
use SilverStripe\Core\ClassInfo;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Injector\Injector;
Expand Down Expand Up @@ -2841,49 +2842,55 @@ public function setField($fieldName, $val)
// later referenced to update the parent dataobject
if ($val instanceof DBComposite) {
$val->bindTo($this);
$this->record[$fieldName] = $val;
$this->setFieldValue($fieldName, $val);
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
}
// Situation 2: Passing a literal or non-DBField object
} else {
// If this is a proper database field, we shouldn't be getting non-DBField objects
if (is_object($val) && $schema->fieldSpec(static::class, $fieldName)) {
throw new InvalidArgumentException('DataObject::setField: passed an object that is not a DBField');
}
$this->setFieldValue($fieldName, $val);
}
return $this;
}

if (!empty($val) && !is_scalar($val)) {
$dbField = $this->dbObject($fieldName);
if ($dbField && $dbField->scalarValueOnly()) {
throw new InvalidArgumentException(
sprintf(
'DataObject::setField: %s only accepts scalars',
$fieldName
)
);
}
}
private function setFieldValue(string $fieldName, mixed $val): void
{
$schema = static::getSchema();
// If this is a proper database field, we shouldn't be getting non-DBField objects
if (is_object($val) && !($val instanceof DBField) && $schema->fieldSpec(static::class, $fieldName)) {
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
throw new InvalidArgumentException('DataObject::setFieldValue: passed an object that is not a DBField');
}

// if a field is not existing or has strictly changed
if (!array_key_exists($fieldName, $this->original ?? []) || $this->original[$fieldName] !== $val) {
// TODO Add check for php-level defaults which are not set in the db
// TODO Add check for hidden input-fields (readonly) which are not set in the db
// At the very least, the type has changed
$this->changed[$fieldName] = self::CHANGE_STRICT;

if ((!array_key_exists($fieldName, $this->original ?? []) && $val)
|| (array_key_exists($fieldName, $this->original ?? []) && $this->original[$fieldName] != $val)
) {
// Value has changed as well, not just the type
$this->changed[$fieldName] = self::CHANGE_VALUE;
}
// Value has been restored to its original, remove any record of the change
} elseif (isset($this->changed[$fieldName])) {
unset($this->changed[$fieldName]);
if (!empty($val) && !is_scalar($val)) {
$dbField = $this->dbObject($fieldName);
if ($dbField && $dbField->scalarValueOnly()) {
throw new InvalidArgumentException(
sprintf(
'DataObject::setFieldValue: %s only accepts scalars',
$fieldName
)
);
}
}

// Value is saved regardless, since the change detection relates to the last write
$this->record[$fieldName] = $val;
// if a field is not existing or has strictly changed
if (!array_key_exists($fieldName, $this->original ?? []) || $this->original[$fieldName] !== $val) {
// TODO Add check for php-level defaults which are not set in the db
// TODO Add check for hidden input-fields (readonly) which are not set in the db
// At the very least, the type has changed
$this->changed[$fieldName] = self::CHANGE_STRICT;

if ((!array_key_exists($fieldName, $this->original ?? []) && $val)
|| (array_key_exists($fieldName, $this->original ?? []) && $this->original[$fieldName] != $val)
) {
// Value has changed as well, not just the type
$this->changed[$fieldName] = self::CHANGE_VALUE;
}
// Value has been restored to its original, remove any record of the change
} elseif (isset($this->changed[$fieldName])) {
unset($this->changed[$fieldName]);
}
return $this;

// Value is saved regardless, since the change detection relates to the last write
$this->record[$fieldName] = $val;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/ORM/FieldType/DBBoolean.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public function saveInto($dataObject)
{
$fieldName = $this->name;
if ($fieldName) {
$dataObject->$fieldName = ($this->value) ? 1 : 0;
$dataObject->setField($fieldName, $this->value ? 1 : 0);
} else {
$class = static::class;
throw new \RuntimeException("DBField::saveInto() Called on a nameless '$class' object");
Expand Down
3 changes: 2 additions & 1 deletion src/ORM/FieldType/DBDecimal.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ public function saveInto($dataObject)
$fieldName = $this->name;

if ($fieldName) {
$dataObject->$fieldName = (float)preg_replace('/[^0-9.\-\+]/', '', $this->value ?? '');
$value = (float) preg_replace('/[^0-9.\-\+]/', '', $this->value ?? '');
$dataObject->setField($fieldName, $value);
} else {
throw new \UnexpectedValueException(
"DBField::saveInto() Called on a nameless '" . static::class . "' object"
Expand Down
2 changes: 1 addition & 1 deletion src/ORM/FieldType/DBField.php
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ public function saveInto($dataObject)
"DBField::saveInto() Called on a nameless '" . static::class . "' object"
);
}
$dataObject->$fieldName = $this->value;
$dataObject->setField($fieldName, $this->value);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/ORM/FieldType/DBPercentage.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function saveInto($dataObject)

$fieldName = $this->name;
if ($fieldName && $dataObject->$fieldName > 1.0) {
$dataObject->$fieldName = 1.0;
$dataObject->setField($fieldName, 1.0);
}
}
}
26 changes: 20 additions & 6 deletions src/View/ViewableData.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ class ViewableData implements IteratorAggregate
*/
private static $casting_cache = [];

/**
* Acts as a PHP 8.2+ compliant replacement for dynamic properties
*/
private array $data = [];
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved

// -----------------------------------------------------------------------------------------------------------------

/**
Expand Down Expand Up @@ -191,7 +196,7 @@ public function getFailover()
*/
public function hasField($field)
{
return property_exists($this, $field ?? '');
return property_exists($this, $field) || isset($this->data[$field]);
}

/**
Expand All @@ -202,7 +207,10 @@ public function hasField($field)
*/
public function getField($field)
{
return $this->$field;
if (property_exists($this, $field)) {
return $this->$field;
}
return $this->data[$field];
}

/**
Expand All @@ -215,7 +223,13 @@ public function getField($field)
public function setField($field, $value)
{
$this->objCacheClear();
$this->$field = $value;
// prior to PHP 8.2 support ViewableData::setField() simply used `$this->field = $value;`
// so the following logic essentially mimics this behaviour, though without the use
// of now deprecated dynamic properties
if (property_exists($this, $field)) {
$this->$field = $value;
}
$this->data[$field] = $value;
return $this;
}

Expand Down Expand Up @@ -509,14 +523,14 @@ public function obj($fieldName, $arguments = [], $cache = false, $cacheName = nu
* A simple wrapper around {@link ViewableData::obj()} that automatically caches the result so it can be used again
* without re-running the method.
*
* @param string $field
* @param string $fieldName
* @param array $arguments
* @param string $identifier an optional custom cache identifier
* @return Object|DBField
*/
public function cachedCall($field, $arguments = [], $identifier = null)
public function cachedCall($fieldName, $arguments = [], $identifier = null)
{
return $this->obj($field, $arguments, true, $identifier);
return $this->obj($fieldName, $arguments, true, $identifier);
}

/**
Expand Down
17 changes: 12 additions & 5 deletions src/View/ViewableData_Customised.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,26 @@ public function hasMethod($method)
return $this->customised->hasMethod($method) || $this->original->hasMethod($method);
}

public function cachedCall($field, $arguments = null, $identifier = null)
public function cachedCall($fieldName, $arguments = null, $identifier = null)
{
if ($this->customised->hasMethod($field) || $this->customised->hasField($field)) {
return $this->customised->cachedCall($field, $arguments, $identifier);
if ($this->customisedHas($fieldName)) {
return $this->customised->cachedCall($fieldName, $arguments, $identifier);
}
return $this->original->cachedCall($field, $arguments, $identifier);
return $this->original->cachedCall($fieldName, $arguments, $identifier);
}

public function obj($fieldName, $arguments = null, $cache = false, $cacheName = null)
{
if ($this->customised->hasField($fieldName) || $this->customised->hasMethod($fieldName)) {
if ($this->customisedHas($fieldName)) {
return $this->customised->obj($fieldName, $arguments, $cache, $cacheName);
}
return $this->original->obj($fieldName, $arguments, $cache, $cacheName);
}

private function customisedHas(string $fieldName): bool
{
return property_exists($this->customised, $fieldName) ||
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
$this->customised->hasField($fieldName) ||
$this->customised->hasMethod($fieldName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,7 @@

class AnotherService
{
public $config_property;

public $filters = [];
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

class SampleService
{
public $auto;
public $constructorVarOne;
public $constructorVarTwo;

Expand Down
1 change: 1 addition & 0 deletions tests/php/Core/Injector/InjectorTest/TestObject.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

class TestObject implements TestOnly
{
public $auto;

public $sampleService;

Expand Down
2 changes: 2 additions & 0 deletions tests/php/Dev/FixtureBlueprintTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ class FixtureBlueprintTest extends SapphireTest

protected $usesDatabase = true;

private int $_called = 0;

protected static $extra_dataobjects = [
TestDataObject::class,
DataObjectRelation::class,
Expand Down
19 changes: 9 additions & 10 deletions tests/php/Forms/GridField/GridFieldFilterHeaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,18 +138,17 @@ public function testHandleActionReset()
public function testGetSearchForm()
{
$searchForm = $this->component->getSearchForm($this->gridField);

$this->assertTrue($searchForm instanceof Form);
$this->assertEquals('Search__q', $searchForm->fields[0]->Name);
$this->assertEquals('Search__Name', $searchForm->fields[1]->Name);
$this->assertEquals('Search__City', $searchForm->fields[2]->Name);
$this->assertEquals('Search__Cheerleader__Hat__Colour', $searchForm->fields[3]->Name);
$fields = $searchForm->Fields()->toArray();
$this->assertEquals('Search__q', $fields[0]->Name);
$this->assertEquals('Search__Name', $fields[1]->Name);
$this->assertEquals('Search__City', $fields[2]->Name);
$this->assertEquals('Search__Cheerleader__Hat__Colour', $fields[3]->Name);
$this->assertEquals('TeamsSearchForm', $searchForm->Name);
$this->assertEquals('cms-search-form', $searchForm->extraClasses['cms-search-form']);

foreach ($searchForm->fields as $field) {
$this->assertEquals('stacked', $field->extraClasses['stacked']);
$this->assertEquals('no-change-track', $field->extraClasses['no-change-track']);
$this->assertTrue($searchForm->hasExtraClass('cms-search-form'));
foreach ($fields as $field) {
$this->assertTrue($field->hasExtraClass('stacked'));
$this->assertTrue($field->hasExtraClass('no-change-track'));
}
}

Expand Down
4 changes: 0 additions & 4 deletions tests/php/ORM/DataObjectTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -826,10 +826,6 @@ public function testHasOneAsField()
$team1->Captain = $captain2;
$team1->write();
$this->assertEquals($captain2->ID, $team1->Captain->ID);

// Setter: Custom data (required by DataDifferencer)
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
$team1->Captain = DBField::create_field('HTMLFragment', '<p>No captain</p>');
$this->assertEquals('<p>No captain</p>', $team1->Captain);
}

/**
Expand Down
Loading