API Convert fieldSpec options to bitwise operators (#6161)

Fixes #6159

Forms/GridField/GridFieldSortableHeader.php

@@ -3,6 +3,7 @@
 namespace SilverStripe\Forms\GridField;
 
 use SilverStripe\Forms\LiteralField;
+use SilverStripe\ORM\DataObjectSchema;
 use SilverStripe\ORM\Sortable;
 use SilverStripe\ORM\ArrayList;
 use SilverStripe\ORM\SS_List;
@@ -140,7 +141,10 @@ class GridFieldSortableHeader implements GridField_HTMLProvider, GridField_DataM
 					} elseif(method_exists($tmpItem, 'hasMethod') && $tmpItem->hasMethod($methodName)) {
 						// The part is a relation name, so get the object/list from it
 						$tmpItem = $tmpItem->$methodName();
-					} elseif ($tmpItem instanceof DataObject && $schema->fieldSpec($tmpItem, $methodName, ['dbOnly'])) {
+					} elseif (
+						$tmpItem instanceof DataObject
+						&& $schema->fieldSpec($tmpItem, $methodName, DataObjectSchema::DB_ONLY)
+					) {
 						// Else, if we've found a database field at the end of the chain, we can sort on it.
 						// If a method is applied further to this field (E.g. 'Cost.Currency') then don't try to sort.
 						$allowSort = $idx === sizeof($parts) - 1;

ORM/DataObject.php

@@ -343,7 +343,10 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 		// Identify fields that should be lazy loaded, but only on existing records
 		if(!empty($record['ID'])) {
 			// Get all field specs scoped to class for later lazy loading
-			$fields = static::getSchema()->fieldSpecs(static::class, ['includeClass', 'dbOnly']);
+			$fields = static::getSchema()->fieldSpecs(
+				static::class,
+				DataObjectSchema::INCLUDE_CLASS | DataObjectSchema::DB_ONLY
+			);
 			foreach($fields as $field => $fieldSpec) {
 				$fieldClass = strtok($fieldSpec, ".");
 				if(!array_key_exists($field, $record)) {
@@ -1194,7 +1197,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 			}
 
 			// Ensure this field pertains to this table
-			$specification = $schema->fieldSpec($class, $fieldName, ['dbOnly', 'uninherited']);
+			$specification = $schema->fieldSpec($class, $fieldName, DataObjectSchema::DB_ONLY | DataObjectSchema::UNINHERITED);
 			if (!$specification) {
 				continue;
 			}
@@ -2403,7 +2406,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	 * @return boolean
 	 */
 	public function hasDatabaseField($field) {
-		$spec = static::getSchema()->fieldSpec(static::class, $field, ['dbOnly']);
+		$spec = static::getSchema()->fieldSpec(static::class, $field, DataObjectSchema::DB_ONLY);
 		return !empty($spec);
 	}
 
@@ -2587,7 +2590,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	 */
 	public function dbObject($fieldName) {
 		// Check for field in DB
-		$helper = static::getSchema()->fieldSpec(static::class, $fieldName, ['includeClass']);
+		$helper = static::getSchema()->fieldSpec(static::class, $fieldName, DataObjectSchema::INCLUDE_CLASS);
 		if(!$helper) {
 			return null;
 		}

ORM/DataObjectSchema.php

@@ -137,21 +137,41 @@ class DataObjectSchema {
 		return $this->tableName($this->baseDataClass($class));
 	}
 
+	/**
+	 * fieldSpec should exclude virtual fields (such as composite fields), and only include fields with a db column.
+	 */
+	const DB_ONLY = 1;
+
+	/**
+	 * fieldSpec should only return fields that belong to this table, and not any ancestors
+	 */
+	const UNINHERITED = 2;
+
+	/**
+	 * fieldSpec should prefix all field specifications with the class name in RecordClass.Column(spec) format.
+	 */
+	const INCLUDE_CLASS = 4;
+
 	/**
 	 * Get all DB field specifications for a class, including ancestors and composite fields.
 	 *
 	 * @param string|DataObject $classOrInstance
-	 * @param array $options Array of options. Specify any number of the below:
-	 *  - `uninherited`: Set to true to limit to only this table
-	 *  - `dbOnly`: Exclude virtual fields (such as composite fields), and only include fields with a db column.
-	 *  - `includeClass`: If true prefix the field specification with the class name in RecordClass.Column(spec) format.
-	 * @return array
+	 * @param int $options Bitmask of options
+	 *  - UNINHERITED Limit to only this table
+	 *  - DB_ONLY Exclude virtual fields (such as composite fields), and only include fields with a db column.
+	 *  - INCLUDE_CLASS Prefix the field specification with the class name in RecordClass.Column(spec) format.
+	 * @return array List of fields, where the key is the field name and the value is the field specification.
 	 */
-	public function fieldSpecs($classOrInstance, $options = []) {
+	public function fieldSpecs($classOrInstance, $options = 0) {
 		$class = ClassInfo::class_name($classOrInstance);
-		$uninherited = !empty($options['uninherited']) || in_array('uninherited', $options);
-		$dbOnly = !empty($options['dbOnly']) || in_array('dbOnly', $options);
-		$includeClass = !empty($options['includeClass']) || in_array('includeClass', $options);
+
+		// Validate options
+		if (!is_int($options)) {
+			throw new InvalidArgumentException("Invalid options " . var_export($options, true));
+		}
+		$uninherited = ($options & self::UNINHERITED) === self::UNINHERITED;
+		$dbOnly = ($options & self::DB_ONLY) === self::DB_ONLY;
+		$includeClass = ($options & self::INCLUDE_CLASS) === self::INCLUDE_CLASS;
 
 		// Walk class hierarchy
 		$db = [];
@@ -180,15 +200,15 @@ class DataObjectSchema {
 	 * Get specifications for a single class field
 	 *
 	 * @param string|DataObject $classOrInstance Name or instance of class
-	 * @param string $fieldName
-	 * @param array $options Array of options. Specify any number of the below:
-	 *  - `uninherited`: Set to true to limit to only this table
-	 *  - `dbOnly`: Exclude virtual fields (such as composite fields), and only include fields with a db column.
-	 *  - `includeClass`: If true prefix the field specification with the class name in RecordClass.Column(spec) format.
+	 * @param string $fieldName Name of field to retrieve
+	 * @param int $options Bitmask of options
+	 *  - UNINHERITED Limit to only this table
+	 *  - DB_ONLY Exclude virtual fields (such as composite fields), and only include fields with a db column.
+	 *  - INCLUDE_CLASS Prefix the field specification with the class name in RecordClass.Column(spec) format.
 	 * @return string|null Field will be a string in FieldClass(args) format, or
-	 * RecordClass.FieldClass(args) format if $includeClass is true. Will be null if no field is found.
+	 * RecordClass.FieldClass(args) format if using INCLUDE_CLASS. Will be null if no field is found.
 	 */
-	public function fieldSpec($classOrInstance, $fieldName, $options = []) {
+	public function fieldSpec($classOrInstance, $fieldName, $options = 0) {
 		$specs = $this->fieldSpecs($classOrInstance, $options);
 		return isset($specs[$fieldName]) ? $specs[$fieldName] : null;
 	}
@@ -473,8 +493,7 @@ class DataObjectSchema {
 
 	/**
 	 * Return information about a specific many_many component. Returns a numeric array.
-	 * The first item in the array will be the class name of the relation: either
-	 * RELATION_MANY_MANY or RELATION_MANY_MANY_THROUGH constant value.
+	 * The first item in the array will be the class name of the relation.
 	 *
 	 * Standard many_many return type is:
 	 *

docs/en/04_Changelogs/4.0.0.md

@@ -922,7 +922,7 @@ A very small number of methods were chosen for deprecation, and will be removed
 * Removed `DataObject::validateModelDefinitions`. Relations are now validated within `DataObjectSchema`
 * Removed `DataObject` methods `hasOwnTableDatabaseField`, `has_own_table_database_field` and
   `hasDatabaseFields` are superceded by `DataObjectSchema::fieldSpec`.
-  Use `$schema->fieldSpec($class, $field, ['dbOnly', 'uninherited'])`.
+  Use `$schema->fieldSpec($class, $field, DataObjectSchema::DB_ONLY | DataObjectSchema::UNINHERITED )`.
   Exclude `uninherited` option to search all tables in the class hierarchy.
 * Removed `DataObject::is_composite_field`. Use `DataObjectSchema::compositeField` instead.
 * Removed `DataObject::custom_database_fields`. Use `DataObjectSchema::databaseFields`

tests/model/DataObjectSchemaTest.php

@@ -3,7 +3,7 @@
 use SilverStripe\ORM\DataObject;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Dev\TestOnly;
-
+use SilverStripe\ORM\DataObjectSchema;
 
 /**
  * Tests schema inspection of DataObjects
@@ -295,6 +295,68 @@ class DataObjectSchemaTest extends SapphireTest
 
 	}
 
+	public function testFieldSpec() {
+		$schema = DataObject::getSchema();
+		$this->assertEquals(
+			[
+				'ID' => 'PrimaryKey',
+				'ClassName' => 'DBClassName',
+				'LastEdited' => 'DBDatetime',
+				'Created' => 'DBDatetime',
+				'Title' => 'Varchar',
+				'Description' => 'Varchar',
+				'MoneyFieldCurrency' => 'Varchar(3)',
+				'MoneyFieldAmount' => 'Decimal(19,4)',
+				'MoneyField' => 'Money',
+			],
+			$schema->fieldSpecs(DataObjectSchemaTest_HasFields::class)
+		);
+		$this->assertEquals(
+			[
+				'ID' => 'DataObjectSchemaTest_HasFields.PrimaryKey',
+				'ClassName' => 'DataObjectSchemaTest_BaseDataClass.DBClassName',
+				'LastEdited' => 'DataObjectSchemaTest_BaseDataClass.DBDatetime',
+				'Created' => 'DataObjectSchemaTest_BaseDataClass.DBDatetime',
+				'Title' => 'DataObjectSchemaTest_BaseDataClass.Varchar',
+				'Description' => 'DataObjectSchemaTest_HasFields.Varchar',
+				'MoneyFieldCurrency' => 'DataObjectSchemaTest_HasFields.Varchar(3)',
+				'MoneyFieldAmount' => 'DataObjectSchemaTest_HasFields.Decimal(19,4)',
+				'MoneyField' => 'DataObjectSchemaTest_HasFields.Money',
+			],
+			$schema->fieldSpecs(DataObjectSchemaTest_HasFields::class, DataObjectSchema::INCLUDE_CLASS)
+		);
+		// DB_ONLY excludes composite field MoneyField
+		$this->assertEquals(
+			[
+				'ID' => 'DataObjectSchemaTest_HasFields.PrimaryKey',
+				'ClassName' => 'DataObjectSchemaTest_BaseDataClass.DBClassName',
+				'LastEdited' => 'DataObjectSchemaTest_BaseDataClass.DBDatetime',
+				'Created' => 'DataObjectSchemaTest_BaseDataClass.DBDatetime',
+				'Title' => 'DataObjectSchemaTest_BaseDataClass.Varchar',
+				'Description' => 'DataObjectSchemaTest_HasFields.Varchar',
+				'MoneyFieldCurrency' => 'DataObjectSchemaTest_HasFields.Varchar(3)',
+				'MoneyFieldAmount' => 'DataObjectSchemaTest_HasFields.Decimal(19,4)'
+			],
+			$schema->fieldSpecs(
+				DataObjectSchemaTest_HasFields::class,
+				DataObjectSchema::INCLUDE_CLASS | DataObjectSchema::DB_ONLY
+			)
+		);
+
+		// Use all options at once
+		$this->assertEquals(
+			[
+				'ID' => 'DataObjectSchemaTest_HasFields.PrimaryKey',
+				'Description' => 'DataObjectSchemaTest_HasFields.Varchar',
+				'MoneyFieldCurrency' => 'DataObjectSchemaTest_HasFields.Varchar(3)',
+				'MoneyFieldAmount' => 'DataObjectSchemaTest_HasFields.Decimal(19,4)',
+			],
+			$schema->fieldSpecs(
+				DataObjectSchemaTest_HasFields::class,
+				DataObjectSchema::INCLUDE_CLASS | DataObjectSchema::DB_ONLY | DataObjectSchema::UNINHERITED
+			)
+		);
+	}
 
 	/**
 	 * @covers SilverStripe\ORM\DataObjectSchema::baseDataClass()
@@ -315,7 +377,9 @@ class DataObjectSchemaTest extends SapphireTest
 }
 
 class DataObjectSchemaTest_BaseClass extends DataObject implements TestOnly {
-
+	private static $db = [
+		'Title' => 'Varchar',
+	];
 }
 
 class DataObjectSchemaTest_ChildClass extends DataObjectSchemaTest_BaseClass {
@@ -341,7 +405,8 @@ class DataObjectSchemaTest_NoFields extends DataObjectSchemaTest_BaseDataClass {
 class DataObjectSchemaTest_HasFields extends DataObjectSchemaTest_NoFields {
 
 	private static $db = array(
-		'Description' => 'Varchar'
+		'Description' => 'Varchar',
+		'MoneyField' => 'Money',
 	);
 }
 

tests/model/DataObjectTest.php

@@ -80,13 +80,12 @@ class DataObjectTest extends SapphireTest {
 		// Test with table required
 		$this->assertEquals(
 			'DataObjectTest_TeamComment.Varchar',
-			$schema->fieldSpec(DataObjectTest_TeamComment::class, 'Name', ['includeClass'])
+			$schema->fieldSpec(DataObjectTest_TeamComment::class, 'Name', DataObjectSchema::INCLUDE_CLASS)
 		);
 		$this->assertEquals(
 			'DataObjectTest_TeamComment.Text',
-			$schema->fieldSpec(DataObjectTest_TeamComment::class, 'Comment', ['includeClass'])
+			$schema->fieldSpec(DataObjectTest_TeamComment::class, 'Comment', DataObjectSchema::INCLUDE_CLASS)
 		);
-		$obj = new DataObjectTest_ExtendedTeamComment();
 		$dbFields = $schema->fieldSpecs(DataObjectTest_ExtendedTeamComment::class);
 
 		// fixed fields are still included in extended classes

tests/model/VersionedTest.php

@@ -1,6 +1,7 @@
 <?php
 
 use SilverStripe\Dev\Debug;
+use SilverStripe\ORM\DataObjectSchema;
 use SilverStripe\ORM\DB;
 use SilverStripe\ORM\HasManyList;
 use SilverStripe\ORM\ManyManyList;
@@ -409,25 +410,25 @@ class VersionedTest extends SapphireTest {
 		$schema = DataObject::getSchema();
 
 		$this->assertNull(
-			$schema->fieldSpec(DataObject::class, 'Version', ['uninherited']),
+			$schema->fieldSpec(DataObject::class, 'Version', DataObjectSchema::UNINHERITED),
 			'Plain models have no version field.'
 		);
 		$this->assertEquals(
 			'Int',
-			$schema->fieldSpec(VersionedTest_DataObject::class, 'Version', ['uninherited']),
+			$schema->fieldSpec(VersionedTest_DataObject::class, 'Version', DataObjectSchema::UNINHERITED),
 			'The versioned ext adds an Int version field.'
 		);
 		$this->assertNull(
-			$schema->fieldSpec(VersionedTest_Subclass::class, 'Version', ['uninherited']),
+			$schema->fieldSpec(VersionedTest_Subclass::class, 'Version', DataObjectSchema::UNINHERITED),
 			'Sub-classes of a versioned model don\'t have a Version field.'
 		);
 		$this->assertNull(
-			$schema->fieldSpec(VersionedTest_AnotherSubclass::class, 'Version', ['uninherited']),
+			$schema->fieldSpec(VersionedTest_AnotherSubclass::class, 'Version', DataObjectSchema::UNINHERITED),
 			'Sub-classes of a versioned model don\'t have a Version field.'
 		);
 		$this->assertEquals(
 			'Varchar(255)',
-			$schema->fieldSpec(VersionedTest_UnversionedWithField::class, 'Version', ['uninherited']),
+			$schema->fieldSpec(VersionedTest_UnversionedWithField::class, 'Version', DataObjectSchema::UNINHERITED),
 			'Models w/o Versioned can have their own Version field.'
 		);
 	}