[com_fields] Fetch all custom field values in one query (#14558)

* Add function to fetch all fields in one shot * Remove the context from the field_values table as it is redundant * Fix notices errors * Cosmetic * Remove the context from the upgrade scripts * Remove notice * No need to double check of null * Remove not needed code * getValue of a single field is using the getValues function * CS * Check if fields ids is empty
joomla · Mar 27, 2017 · 1c52ed3 · 1c52ed3
1 parent 1dda725
commit 1c52ed3
Show file tree

Hide file tree

Showing 10 changed files with 86 additions and 54 deletions.
diff --git a/administrator/components/com_admin/sql/updates/mysql/3.7.0-2016-08-29.sql b/administrator/components/com_admin/sql/updates/mysql/3.7.0-2016-08-29.sql
@@ -66,11 +66,9 @@ CREATE TABLE IF NOT EXISTS `#__fields_groups` (
 
 CREATE TABLE IF NOT EXISTS `#__fields_values` (
   `field_id` int(10) unsigned NOT NULL,
-  `context` varchar(255) NOT NULL,
   `item_id` varchar(255) NOT NULL COMMENT 'Allow references to items which have strings as ids, eg. none db systems.',
   `value` text NOT NULL DEFAULT '',
   KEY (`field_id`),
-  KEY (`context`),
   KEY (`item_id`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 DEFAULT COLLATE=utf8mb4_unicode_ci;
 

diff --git a/administrator/components/com_admin/sql/updates/postgresql/3.7.0-2016-08-29.sql b/administrator/components/com_admin/sql/updates/postgresql/3.7.0-2016-08-29.sql
@@ -79,11 +79,9 @@ CREATE INDEX "#__fields_idx_language" ON "#__fields_groups" ("language");
 CREATE TABLE "#__fields_values" (
 "field_id" bigint DEFAULT 0 NOT NULL,
 "item_id" varchar(255) DEFAULT '' NOT NULL,
-"context" varchar(255) DEFAULT '' NOT NULL,
-"value" text DEFAULT '' NOT NULL 
+"value" text DEFAULT '' NOT NULL
 );
 CREATE INDEX "#__fields_values_idx_field_id" ON "#__fields_values" ("field_id");
-CREATE INDEX "#__fields_values_idx_context" ON "#__fields_values" ("context");
 CREATE INDEX "#__fields_values_idx_item_id" ON "#__fields_values" ("item_id");
 
 INSERT INTO "#__extensions" ("extension_id", "name", "type", "element", "folder", "client_id", "enabled", "access", "protected", "manifest_cache", "params", "custom_data", "system_data", "checked_out", "checked_out_time", "ordering", "state") VALUES

diff --git a/administrator/components/com_admin/sql/updates/sqlazure/3.7.0-2016-08-29.sql b/administrator/components/com_admin/sql/updates/sqlazure/3.7.0-2016-08-29.sql
@@ -121,7 +121,6 @@ SET QUOTED_IDENTIFIER ON;
 
 CREATE TABLE [#__fields_values] (
 	[field_id] [bigint] NOT NULL DEFAULT 1,
-	[context] [nvarchar](255) NOT NULL DEFAULT '',
 	[item_id] [nvarchar](255) NOT NULL DEFAULT '',
 	[value] [nvarchar](max) NOT NULL DEFAULT '',
 ) ON [PRIMARY];
@@ -130,10 +129,6 @@ CREATE NONCLUSTERED INDEX [idx_field_id] ON [#__fields_values](
 	[field_id] ASC)
 WITH (STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF);
 
-CREATE NONCLUSTERED INDEX [idx_context] ON [#__fields_values](
-	[context] ASC)
-WITH (STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF);
-
 CREATE NONCLUSTERED INDEX [idx_item_id] ON [#__fields_values](
 	[item_id] ASC)
 WITH (STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF);

diff --git a/administrator/components/com_fields/helpers/fields.php b/administrator/components/com_fields/helpers/fields.php
@@ -145,6 +145,16 @@ public static function getFields($context, $item = null, $prepareValue = false,
 				self::$fieldCache = JModelLegacy::getInstance('Field', 'FieldsModel', array('ignore_request' => true));
 			}
 
+			$fieldIds = array_map(
+				function($f)
+				{
+					return $f->id;
+				},
+				$fields
+			);
+
+			$fieldValues = self::$fieldCache->getFieldValues($fieldIds, $item->id);
+
 			$new = array();
 
 			foreach ($fields as $key => $original)
@@ -163,12 +173,12 @@ public static function getFields($context, $item = null, $prepareValue = false,
 				{
 					$field->value = $valuesToOverride[$field->id];
 				}
-				else
+				elseif (key_exists($field->id, $fieldValues))
 				{
-					$field->value = self::$fieldCache->getFieldValue($field->id, $field->context, $item->id);
+					$field->value = $fieldValues[$field->id];
 				}
 
-				if ($field->value === '' || $field->value === null)
+				if (!isset($field->value) || $field->value === '')
 				{
 					$field->value = $field->default_value;
 				}
@@ -360,9 +370,7 @@ function categoryHasChanged(element) {
 		$fieldsNode->setAttribute('name', 'com_fields');
 
 		// Organizing the fields according to their group
-		$fieldsPerGroup = array(
-			0 => array()
-		);
+		$fieldsPerGroup = array(0 => array());
 
 		foreach ($fields as $field)
 		{
@@ -498,7 +506,7 @@ function categoryHasChanged(element) {
 
 		foreach ($fields as $field)
 		{
-			$value = $model->getFieldValue($field->id, $field->context, $data->id);
+			$value = $model->getFieldValue($field->id, $data->id);
 
 			if ($value === null)
 			{

diff --git a/administrator/components/com_fields/models/field.php b/administrator/components/com_fields/models/field.php
@@ -449,15 +449,14 @@ public function getForm($data = array(), $loadData = true)
 	 * Setting the value for the gven field id, context and item id.
 	 *
 	 * @param   string  $fieldId  The field ID.
-	 * @param   string  $context  The context.
 	 * @param   string  $itemId   The ID of the item.
 	 * @param   string  $value    The value.
 	 *
 	 * @return  boolean
 	 *
 	 * @since   3.7.0
 	 */
-	public function setFieldValue($fieldId, $context, $itemId, $value)
+	public function setFieldValue($fieldId, $itemId, $value)
 	{
 		$field  = $this->getItem($fieldId);
 		$params = $field->params;
@@ -484,7 +483,7 @@ public function setFieldValue($fieldId, $context, $itemId, $value)
 		}
 		else
 		{
-			$oldValue = $this->getFieldValue($fieldId, $context, $itemId);
+			$oldValue = $this->getFieldValue($fieldId, $itemId);
 			$value    = (array) $value;
 
 			if ($oldValue === null)
@@ -513,7 +512,6 @@ public function setFieldValue($fieldId, $context, $itemId, $value)
 
 			$query->delete($query->qn('#__fields_values'))
 				->where($query->qn('field_id') . ' = ' . (int) $fieldId)
-				->where($query->qn('context') . ' = ' . $query->q($context))
 				->where($query->qn('item_id') . ' = ' . $query->q($itemId));
 
 			$this->getDbo()->setQuery($query)->execute();
@@ -524,7 +522,6 @@ public function setFieldValue($fieldId, $context, $itemId, $value)
 			$newObj = new stdClass;
 
 			$newObj->field_id = (int) $fieldId;
-			$newObj->context  = $context;
 			$newObj->item_id  = $itemId;
 
 			foreach ($value as $v)
@@ -540,11 +537,10 @@ public function setFieldValue($fieldId, $context, $itemId, $value)
 			$updateObj = new stdClass;
 
 			$updateObj->field_id = (int) $fieldId;
-			$updateObj->context  = $context;
 			$updateObj->item_id  = $itemId;
 			$updateObj->value    = reset($value);
 
-			$this->getDbo()->updateObject('#__fields_values', $updateObj, array('field_id', 'context', 'item_id'));
+			$this->getDbo()->updateObject('#__fields_values', $updateObj, array('field_id', 'item_id'));
 		}
 
 		$this->valueCache = array();
@@ -556,48 +552,88 @@ public function setFieldValue($fieldId, $context, $itemId, $value)
 	 * Returning the value for the given field id, context and item id.
 	 *
 	 * @param   string  $fieldId  The field ID.
-	 * @param   string  $context  The context.
 	 * @param   string  $itemId   The ID of the item.
 	 *
 	 * @return  NULL|string
 	 *
 	 * @since  3.7.0
 	 */
-	public function getFieldValue($fieldId, $context, $itemId)
+	public function getFieldValue($fieldId, $itemId)
 	{
-		$key = md5($fieldId . $context . $itemId);
+		$values = $this->getFieldValues(array($fieldId), $itemId);
 
-		if (!key_exists($key, $this->valueCache))
+		if (key_exists($fieldId, $values))
+		{
+			return $values[$fieldId];
+		}
+
+		return null;
+	}
+
+	/**
+	 * Returning the values for the given field ids, context and item id.
+	 *
+	 * @param   array   $fieldIds  The field Ids.
+	 * @param   string  $itemId    The ID of the item.
+	 *
+	 * @return  NULL|array
+	 *
+	 * @since  3.7.0
+	 */
+	public function getFieldValues(array $fieldIds, $itemId)
+	{
+		if (!$fieldIds)
 		{
-			$this->valueCache[$key] = null;
+			return array();
+		}
 
+		// Create a unique key for the cache
+		$key = md5(serialize($fieldIds) . $itemId);
+
+		// Fill the cache when it doesn't exist
+		if (!key_exists($key, $this->valueCache))
+		{
+			// Create the query
 			$query = $this->getDbo()->getQuery(true);
 
-			$query->select($query->qn('value'))
+			$query->select(array($query->qn('field_id'), $query->qn('value')))
 				->from($query->qn('#__fields_values'))
-				->where($query->qn('field_id') . ' = ' . (int) $fieldId)
-				->where($query->qn('context') . ' = ' . $query->q($context))
+				->where($query->qn('field_id') . ' IN (' . implode(',', ArrayHelper::toInteger($fieldIds)) . ')')
 				->where($query->qn('item_id') . ' = ' . $query->q($itemId));
 
+			// Fetch the row from the database
 			$rows = $this->getDbo()->setQuery($query)->loadObjectList();
 
-			if (count($rows) == 1)
-			{
-				$this->valueCache[$key] = array_shift($rows)->value;
-			}
-			elseif (count($rows) > 1)
-			{
-				$data = array();
+			$data = array();
 
-				foreach ($rows as $row)
+			// Fill the data container from the database rows
+			foreach ($rows as $row)
+			{
+				// If there are multiple values for a field, create an array
+				if (key_exists($row->field_id, $data))
 				{
-					$data[] = $row->value;
+					// Transform it to an array
+					if (!is_array($data[$row->field_id]))
+					{
+						$data[$row->field_id] = array($data[$row->field_id]);
+					}
+
+					// Set the value in the array
+					$data[$row->field_id][] = $row->value;
+
+					// Go to the next row, otherwise the value gets overwritten in the data container
+					continue;
 				}
 
-				$this->valueCache[$key] = $data;
+				// Set the value
+				$data[$row->field_id] = $row->value;
 			}
+
+			// Assign it to the internal cache
+			$this->valueCache[$key] = $data;
 		}
 
+		// Return the value from the cache
 		return $this->valueCache[$key];
 	}
 
@@ -613,10 +649,16 @@ public function getFieldValue($fieldId, $context, $itemId)
 	 */
 	public function cleanupValues($context, $itemId)
 	{
+		// Delete with inner join is not possible so we need to do a subquery
+		$fieldsQuery = $this->getDbo()->getQuery(true);
+		$fieldsQuery->select($fieldsQuery->qn('id'))
+			->from($fieldsQuery->qn('#__fields'))
+			->where($fieldsQuery->qn('context') . ' = ' . $fieldsQuery->q($context));
+
 		$query = $this->getDbo()->getQuery(true);
 
 		$query->delete($query->qn('#__fields_values'))
-			->where($query->qn('context') . ' = ' . $query->q($context))
+			->where($query->qn('field_id') . ' IN (' . $fieldsQuery . ')')
 			->where($query->qn('item_id') . ' = ' . $query->q($itemId));
 
 		$this->getDbo()->setQuery($query)->execute();

diff --git a/administrator/components/com_fields/models/fields.php b/administrator/components/com_fields/models/fields.php
@@ -109,7 +109,7 @@ protected function getStoreId($id = '')
 		$id .= ':' . serialize($this->getState('filter.assigned_cat_ids'));
 		$id .= ':' . $this->getState('filter.state');
 		$id .= ':' . $this->getState('filter.group_id');
-		$id .= ':' . print_r($this->getState('filter.language'), true);
+		$id .= ':' . serialize($this->getState('filter.language'));
 
 		return parent::getStoreId($id);
 	}

diff --git a/installation/sql/mysql/joomla.sql b/installation/sql/mysql/joomla.sql
@@ -739,11 +739,9 @@ CREATE TABLE IF NOT EXISTS `#__fields_groups` (
 
 CREATE TABLE IF NOT EXISTS `#__fields_values` (
   `field_id` int(10) unsigned NOT NULL,
-  `context` varchar(255) NOT NULL,
   `item_id` varchar(255) NOT NULL COMMENT 'Allow references to items which have strings as ids, eg. none db systems.',
   `value` text NOT NULL DEFAULT '',
   KEY (`field_id`),
-  KEY (`context`),
   KEY (`item_id`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 DEFAULT COLLATE=utf8mb4_unicode_ci;
 

diff --git a/installation/sql/postgresql/joomla.sql b/installation/sql/postgresql/joomla.sql
@@ -747,11 +747,9 @@ CREATE INDEX "#__fields_groups_idx_language" ON "#__fields_groups" ("language");
 CREATE TABLE "#__fields_values" (
 "field_id" bigint DEFAULT 0 NOT NULL,
 "item_id" varchar(255) DEFAULT '' NOT NULL,
-"context" varchar(255) DEFAULT '' NOT NULL,
 "value" text DEFAULT '' NOT NULL
 );
 CREATE INDEX "#__fields_values_idx_field_id" ON "#__fields_values" ("field_id");
-CREATE INDEX "#__fields_values_idx_context" ON "#__fields_values" ("context");
 CREATE INDEX "#__fields_values_idx_item_id" ON "#__fields_values" ("item_id");
 
 --

diff --git a/installation/sql/sqlazure/joomla.sql b/installation/sql/sqlazure/joomla.sql
@@ -999,7 +999,6 @@ WITH (STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, O
 
 CREATE TABLE "#__fields_values" (
   "field_id" bigint NOT NULL DEFAULT 1,
-  "context" nvarchar(255) NOT NULL DEFAULT '',
   "item_id" nvarchar(255) NOT NULL DEFAULT '',
   "value" nvarchar(max) NOT NULL DEFAULT '',
 ) ON [PRIMARY];
@@ -1008,10 +1007,6 @@ CREATE NONCLUSTERED INDEX "idx_field_id" ON "#__fields_values" (
   "field_id" ASC)
 WITH (STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF);
 
-CREATE NONCLUSTERED INDEX "idx_context" ON "#__fields_values" (
-  "context" ASC)
-WITH (STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF);
-
 CREATE NONCLUSTERED INDEX "idx_item_id" ON "#__fields_values" (
   "item_id" ASC)
 WITH (STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF);

diff --git a/plugins/system/fields/fields.php b/plugins/system/fields/fields.php
@@ -86,7 +86,7 @@ public function onContentAfterSave($context, $item, $isNew, $data = array())
 			$value = key_exists($field->alias, $fieldsData) ? $fieldsData[$field->alias] : null;
 
 			// Setting the value for the field and the item
-			$model->setFieldValue($field->id, $context, $item->id, $value);
+			$model->setFieldValue($field->id, $id, $value);
 		}
 
 		return true;