diff --git a/core/components/minishop3/src/Services/Category/CategoryProductsListService.php b/core/components/minishop3/src/Services/Category/CategoryProductsListService.php index 78617e61..7f0341df 100644 --- a/core/components/minishop3/src/Services/Category/CategoryProductsListService.php +++ b/core/components/minishop3/src/Services/Category/CategoryProductsListService.php @@ -9,12 +9,15 @@ use MiniShop3\Model\msProductData; use MiniShop3\Model\msProductOption; use MiniShop3\Services\Grid\GridOptionColumnResolver; +use MiniShop3\Services\Grid\GridRelationColumnResolver; use MiniShop3\Services\Grid\OptionColumnSpec; +use MiniShop3\Services\Grid\RelationColumnSpec; use MODX\Revolution\modX; use xPDO\Om\xPDOQuery; /** - * Category products grid: SQL list query with optional option columns (JOIN + GROUP BY + GROUP_CONCAT). + * Category products grid: SQL list query with optional option columns (JOIN + GROUP BY + GROUP_CONCAT) + * and relation columns (JOIN + SELECT from related tables). * * Multi-value options appear as one string (MySQL GROUP_CONCAT). For very large sets, server * `group_concat_max_len` may truncate the result. @@ -43,16 +46,17 @@ public function getPage( string $sortDir, ): array { $optionSpecs = GridOptionColumnResolver::resolve($gridFields); + $relationSpecs = GridRelationColumnResolver::resolve($this->modx, $gridFields); - $c = $this->buildProductListQuery($categoryId, $params, $nested, $optionSpecs); + $c = $this->buildProductListQuery($categoryId, $params, $nested, $optionSpecs, $relationSpecs); - $countQuery = $this->buildProductListQuery($categoryId, $params, $nested, $optionSpecs); + $countQuery = $this->buildProductListQuery($categoryId, $params, $nested, $optionSpecs, $relationSpecs); $countQuery->select('COUNT(DISTINCT msProduct.id)'); $countQuery->prepare(); $countQuery->stmt->execute(); $total = (int) $countQuery->stmt->fetchColumn(); - $sortField = $this->mapSortField($sortBy, $optionSpecs); + $sortField = $this->mapSortField($sortBy, $optionSpecs, $relationSpecs); $c->sortby($sortField, $sortDir); $c->limit($limit, $start); @@ -73,6 +77,9 @@ public function getPage( foreach ($optionSpecs as $spec) { $selectParts[] = $this->aggregateOptionValueSql($spec->alias) . " AS `{$spec->fieldName}`"; } + foreach ($relationSpecs as $spec) { + $selectParts[] = $spec->selectExpression(); + } // xPDOQuery::select() declares string, accepts both at runtime but PHPStan is strict. $c->select(implode(', ', $selectParts)); if ($optionSpecs !== []) { @@ -83,9 +90,10 @@ public function getPage( $rows = $c->stmt->execute() ? $c->stmt->fetchAll(\PDO::FETCH_ASSOC) : []; $optionFieldNames = array_map(static fn (OptionColumnSpec $s) => $s->fieldName, $optionSpecs); + $relationFieldNames = array_map(static fn (RelationColumnSpec $s) => $s->fieldName, $relationSpecs); $results = []; foreach ($rows as $row) { - $results[] = $this->formatProductRow($row, $nested, $optionFieldNames); + $results[] = $this->formatProductRow($row, $nested, $optionFieldNames, $relationFieldNames); } return [ @@ -103,15 +111,21 @@ private function aggregateOptionValueSql(string $alias): string } /** - * @param list $optionSpecs + * @param list $optionSpecs + * @param list $relationSpecs */ - private function mapSortField(string $sortBy, array $optionSpecs): string + private function mapSortField(string $sortBy, array $optionSpecs, array $relationSpecs): string { foreach ($optionSpecs as $spec) { if ($spec->fieldName === $sortBy) { return $this->aggregateOptionValueSql($spec->alias); } } + foreach ($relationSpecs as $spec) { + if ($spec->fieldName === $sortBy) { + return $spec->sortExpression(); + } + } $productFields = ['id', 'pagetitle', 'menuindex', 'published', 'createdon', 'editedon']; if (in_array($sortBy, $productFields, true)) { return "msProduct.{$sortBy}"; @@ -125,10 +139,16 @@ private function mapSortField(string $sortBy, array $optionSpecs): string } /** - * @param list $optionSpecs + * @param list $optionSpecs + * @param list $relationSpecs */ - private function buildProductListQuery(int $categoryId, array $params, bool $nested, array $optionSpecs): xPDOQuery - { + private function buildProductListQuery( + int $categoryId, + array $params, + bool $nested, + array $optionSpecs, + array $relationSpecs, + ): xPDOQuery { $query = trim((string) ($params['query'] ?? '')); $c = $this->modx->newQuery(msProduct::class); $c->innerJoin(msProductData::class, 'Data', 'msProduct.id = Data.id'); @@ -143,6 +163,10 @@ private function buildProductListQuery(int $categoryId, array $params, bool $nes ); } + foreach (GridRelationColumnResolver::uniqueJoins($relationSpecs) as $spec) { + $c->leftJoin($spec->modelClass, $spec->alias, $spec->joinCondition()); + } + $c->where(['msProduct.class_key' => msProduct::class]); if ($nested) { @@ -240,12 +264,17 @@ private function getChildCategories(int $parentId): array } /** - * @param list $optionFieldNames Allowed option field names (whitelist) + * @param list $optionFieldNames Allowed option field names (whitelist) + * @param list $relationFieldNames Allowed relation field names (whitelist) * * @return array */ - private function formatProductRow(array $row, bool $nested, array $optionFieldNames): array - { + private function formatProductRow( + array $row, + bool $nested, + array $optionFieldNames, + array $relationFieldNames, + ): array { $id = (int) $row['id']; $data = [ 'id' => $id, @@ -273,9 +302,9 @@ private function formatProductRow(array $row, bool $nested, array $optionFieldNa 'preview_url' => $this->modx->makeUrl($id, '', '', 'full'), ]; - $allowedOptionFields = array_flip($optionFieldNames); + $allowedExtraFields = array_flip(array_merge($optionFieldNames, $relationFieldNames)); foreach ($row as $key => $value) { - if (!array_key_exists($key, $data) && isset($allowedOptionFields[$key])) { + if (!array_key_exists($key, $data) && isset($allowedExtraFields[$key])) { $data[$key] = $value; } } diff --git a/core/components/minishop3/src/Services/Grid/GridColumnRules.php b/core/components/minishop3/src/Services/Grid/GridColumnRules.php new file mode 100644 index 00000000..68459dc1 --- /dev/null +++ b/core/components/minishop3/src/Services/Grid/GridColumnRules.php @@ -0,0 +1,42 @@ +> $gridFields + * + * @return list + */ + public static function resolve(modX $modx, array $gridFields): array + { + /** @var GridConfigService|null $gridConfig */ + $gridConfig = $modx->services->get('ms3_grid_config'); + if (!$gridConfig) { + return []; + } + + $relationGroups = $gridConfig->extractRelationFields($gridFields); + $out = []; + + foreach ($relationGroups as $group) { + foreach ($group['fields'] as $fieldDef) { + $spec = RelationColumnSpec::fromGroupField($group, $fieldDef); + if ($spec !== null) { + $out[] = $spec; + } + } + } + + return $out; + } + + /** + * Unique JOIN specs by alias (one JOIN per table+foreignKey group). + * + * @param list $specs + * + * @return list + */ + public static function uniqueJoins(array $specs): array + { + $seen = []; + $joins = []; + + foreach ($specs as $spec) { + if (isset($seen[$spec->alias])) { + continue; + } + $seen[$spec->alias] = true; + $joins[] = $spec; + } + + return $joins; + } +} diff --git a/core/components/minishop3/src/Services/Grid/OptionColumnSpec.php b/core/components/minishop3/src/Services/Grid/OptionColumnSpec.php index 299e311d..08ca339c 100644 --- a/core/components/minishop3/src/Services/Grid/OptionColumnSpec.php +++ b/core/components/minishop3/src/Services/Grid/OptionColumnSpec.php @@ -9,28 +9,11 @@ */ final readonly class OptionColumnSpec { - /** Same rule as {@see \MiniShop3\Services\GridConfigService::validateOptionConfig()} */ - private const OPTION_KEY_PATTERN = '/^[a-z0-9_]+$/i'; + /** Same rule as {@see GridColumnRules::SQL_IDENTIFIER_PATTERN} */ + private const OPTION_KEY_PATTERN = GridColumnRules::SQL_IDENTIFIER_PATTERN; /** Same rule as OPTION_KEY_PATTERN; fieldName also lands in `SELECT … AS \`{name}\``. */ - private const FIELD_NAME_PATTERN = '/^[a-z0-9_]+$/i'; - - /** - * Builtin product / product-data column names that already appear in SELECT. - * If user picks one as option fieldName, PDO FETCH_ASSOC overwrites the builtin - * with GROUP_CONCAT string → cast in formatProductRow returns 0 / garbage. - * Disallow at spec creation to prevent silent data corruption. - */ - private const RESERVED_NAMES = [ - // modResource - 'id', 'pagetitle', 'longtitle', 'alias', 'parent', 'menuindex', - 'published', 'deleted', 'hidemenu', 'createdon', 'editedon', - // msProductData - 'article', 'price', 'old_price', 'weight', 'image', 'thumb', - 'vendor_id', 'made_in', 'new', 'popular', 'favorite', - // formatProductRow synthetics - 'preview_url', 'category_name', - ]; + private const FIELD_NAME_PATTERN = GridColumnRules::SQL_IDENTIFIER_PATTERN; public function __construct( public string $fieldName, @@ -81,10 +64,7 @@ public static function isValidOptionKey(string $key): bool */ public static function isValidFieldName(string $name): bool { - if (!preg_match(self::FIELD_NAME_PATTERN, $name)) { - return false; - } - return !in_array(strtolower($name), self::RESERVED_NAMES, true); + return GridColumnRules::isValidCategoryProductExtraFieldName($name); } } diff --git a/core/components/minishop3/src/Services/Grid/ProductDataForeignKeys.php b/core/components/minishop3/src/Services/Grid/ProductDataForeignKeys.php new file mode 100644 index 00000000..f40d21d9 --- /dev/null +++ b/core/components/minishop3/src/Services/Grid/ProductDataForeignKeys.php @@ -0,0 +1,32 @@ + */ + private const KEYS = [ + 'vendor_id', + ]; + + /** + * @return list + */ + public static function all(): array + { + return self::KEYS; + } + + public static function contains(string $foreignKey): bool + { + return in_array($foreignKey, self::KEYS, true); + } +} diff --git a/core/components/minishop3/src/Services/Grid/RelationColumnSpec.php b/core/components/minishop3/src/Services/Grid/RelationColumnSpec.php new file mode 100644 index 00000000..3073515d --- /dev/null +++ b/core/components/minishop3/src/Services/Grid/RelationColumnSpec.php @@ -0,0 +1,88 @@ + $group Relation group from GridConfigService::extractRelationFields() + * @param array $fieldDef Single field definition within the group + */ + public static function fromGroupField(array $group, array $fieldDef): ?self + { + $modelClass = $group['modelClass'] ?? null; + if (empty($modelClass)) { + return null; + } + + $foreignKey = (string) ($group['foreignKey'] ?? ''); + $alias = (string) ($group['alias'] ?? ''); + $fieldName = (string) ($fieldDef['name'] ?? ''); + $displayField = (string) ($fieldDef['displayField'] ?? ''); + + if ($foreignKey === '' || $alias === '' || $fieldName === '' || $displayField === '') { + return null; + } + + if (!self::isSafeSqlIdentifier($foreignKey) + || !self::isSafeSqlIdentifier($displayField) + || !self::isSafeSqlIdentifier($fieldName) + || !self::isSafeSqlIdentifier($alias) + ) { + return null; + } + + if (!GridColumnRules::isValidCategoryProductExtraFieldName($fieldName)) { + return null; + } + + return new self( + $fieldName, + $displayField, + (string) $modelClass, + $alias, + $foreignKey, + self::resolveLocalAlias($foreignKey), + ); + } + + public static function resolveLocalAlias(string $foreignKey): string + { + return ProductDataForeignKeys::contains($foreignKey) ? 'Data' : 'msProduct'; + } + + public function joinCondition(): string + { + return "`{$this->alias}`.id = {$this->localAlias}.{$this->foreignKey}"; + } + + public function selectExpression(): string + { + return "`{$this->alias}`.{$this->displayField} AS `{$this->fieldName}`"; + } + + public function sortExpression(): string + { + return "`{$this->alias}`.{$this->displayField}"; + } + + private static function isSafeSqlIdentifier(string $name): bool + { + return GridColumnRules::isValidSqlIdentifier($name); + } +} diff --git a/core/components/minishop3/src/Services/GridConfigService.php b/core/components/minishop3/src/Services/GridConfigService.php index e1d913d8..7a94e704 100644 --- a/core/components/minishop3/src/Services/GridConfigService.php +++ b/core/components/minishop3/src/Services/GridConfigService.php @@ -3,6 +3,7 @@ namespace MiniShop3\Services; use MiniShop3\Model\msGridField; +use MiniShop3\Services\Grid\GridColumnRules; use MiniShop3\Services\Grid\OptionColumnSpec; use MODX\Revolution\modX; @@ -341,7 +342,11 @@ public function addField(string $gridKey, array $data): array break; case 'relation': - $validation = $this->validateRelationConfig($config); + $validation = $this->validateRelationConfig( + $config, + (string) ($data['field_name'] ?? ''), + $gridKey + ); if (!$validation['success']) { return $validation; } @@ -479,11 +484,14 @@ public function updateField(string $gridKey, string $fieldName, array $data): ar } break; case 'relation': - $validation = $this->validateRelationConfig($config); + $validation = $this->validateRelationConfig( + $config, + (string) ($data['field_name'] ?? $fieldName), + $gridKey + ); if (!$validation['success']) { return $validation; } - // Use updated config with resolvedTableName if (isset($validation['config'])) { $config = $validation['config']; } @@ -595,7 +603,7 @@ protected function validateTemplateConfig(array $config): array * @param array $config * @return array */ - protected function validateRelationConfig(array $config): array + protected function validateRelationConfig(array $config, string $fieldName = '', string $gridKey = ''): array { $relation = $config['relation'] ?? []; @@ -611,14 +619,40 @@ protected function validateRelationConfig(array $config): array return ['success' => false, 'message' => 'relation.displayField is required']; } + if (!GridColumnRules::isValidSqlIdentifier((string) $relation['foreignKey'])) { + return ['success' => false, 'message' => 'relation.foreignKey must contain only letters, numbers and underscores']; + } + + if (!GridColumnRules::isValidSqlIdentifier((string) $relation['displayField'])) { + return ['success' => false, 'message' => 'relation.displayField must contain only letters, numbers and underscores']; + } + + if ($gridKey === 'category-products' && $fieldName !== '' + && !GridColumnRules::isValidCategoryProductExtraFieldName($fieldName) + ) { + return [ + 'success' => false, + 'message' => "Field name '{$fieldName}' is not allowed for relation columns: " + . 'it collides with a builtin product column or contains invalid characters. ' + . "Use a distinct name like 'vendor_address' instead.", + ]; + } + // Validate aggregation $aggregation = $relation['aggregation'] ?? null; $allowedAggregations = ['COUNT', 'SUM', 'AVG', 'MIN', 'MAX']; - if ($aggregation !== null && !in_array($aggregation, $allowedAggregations)) { + if ($aggregation !== null && !in_array($aggregation, $allowedAggregations, true)) { return ['success' => false, 'message' => "Invalid aggregation type. Allowed: " . implode(', ', $allowedAggregations)]; } + if ($gridKey === 'category-products' && $aggregation !== null) { + return [ + 'success' => false, + 'message' => 'Relation aggregation is not supported for category-products grid', + ]; + } + // Determine type: model or direct table name $tableOrModel = $relation['table']; $isModel = strpos($tableOrModel, '\\') !== false || strpos($tableOrModel, '::') !== false; @@ -626,6 +660,10 @@ protected function validateRelationConfig(array $config): array if ($isModel) { // This is model class - get table name try { + if (!class_exists($tableOrModel)) { + return ['success' => false, 'message' => "Invalid model class: {$tableOrModel}"]; + } + $tableName = $this->modx->getTableName($tableOrModel); if (empty($tableName)) { @@ -634,6 +672,7 @@ protected function validateRelationConfig(array $config): array // Save resolved table name in config for query usage $config['relation']['resolvedTableName'] = $tableName; + $config['relation']['resolvedModelClass'] = $tableOrModel; } catch (\Exception $e) { return ['success' => false, 'message' => "Invalid model class: {$tableOrModel}. " . $e->getMessage()]; } @@ -644,6 +683,11 @@ protected function validateRelationConfig(array $config): array $tableOrModel = $tablePrefix . $tableOrModel; } $config['relation']['resolvedTableName'] = $tableOrModel; + + $modelClass = $this->resolveModelClass((string) $relation['table']); + if ($modelClass !== null) { + $config['relation']['resolvedModelClass'] = $modelClass; + } } return ['success' => true, 'config' => $config]; @@ -844,15 +888,21 @@ public function extractRelationFields(array $gridFields): array $displayField = $relation['displayField']; $fieldName = $field['name']; + if (!GridColumnRules::isValidSqlIdentifier((string) $foreignKey) + || !GridColumnRules::isValidSqlIdentifier((string) $displayField) + ) { + continue; + } + // Group key: table + foreignKey (same table with same FK = one JOIN) $groupKey = "{$table}_{$foreignKey}"; if (!isset($relationGroups[$groupKey])) { - // Resolve model class from table name - $modelClass = $this->resolveModelClass($table); + $modelClass = $this->resolveModelClassFromRelation($relation); - // Generate unique alias for this JOIN - $alias = "rel_{$table}_{$foreignKey}"; + // Generate unique alias for this JOIN (safe identifier, no namespace chars) + $tableKey = $this->relationTableKey($relation); + $alias = "rel_{$tableKey}_{$foreignKey}"; $relationGroups[$groupKey] = [ 'table' => $table, @@ -881,25 +931,112 @@ public function extractRelationFields(array $gridFields): array */ protected function resolveModelClass(string $tableName): ?string { - // Known MiniShop3 model mappings + $tableName = $this->normalizeTableIdentifier($tableName); + + // Known MiniShop3 model mappings (short class names and physical table names) $modelMap = [ 'msOrder' => 'MiniShop3\\Model\\msOrder', + 'ms3_orders' => 'MiniShop3\\Model\\msOrder', 'msOrderStatus' => 'MiniShop3\\Model\\msOrderStatus', + 'ms3_order_statuses' => 'MiniShop3\\Model\\msOrderStatus', 'msOrderAddress' => 'MiniShop3\\Model\\msOrderAddress', + 'ms3_order_addresses' => 'MiniShop3\\Model\\msOrderAddress', 'msOrderProduct' => 'MiniShop3\\Model\\msOrderProduct', + 'ms3_order_products' => 'MiniShop3\\Model\\msOrderProduct', 'msDelivery' => 'MiniShop3\\Model\\msDelivery', + 'ms3_deliveries' => 'MiniShop3\\Model\\msDelivery', 'msPayment' => 'MiniShop3\\Model\\msPayment', + 'ms3_payments' => 'MiniShop3\\Model\\msPayment', 'msProduct' => 'MiniShop3\\Model\\msProduct', + 'ms3_products' => 'MiniShop3\\Model\\msProduct', 'msProductData' => 'MiniShop3\\Model\\msProductData', + 'ms3_product_data' => 'MiniShop3\\Model\\msProductData', 'msCategory' => 'MiniShop3\\Model\\msCategory', 'msCategoryMember' => 'MiniShop3\\Model\\msCategoryMember', + 'ms3_category_members' => 'MiniShop3\\Model\\msCategoryMember', 'msVendor' => 'MiniShop3\\Model\\msVendor', + 'ms3_vendors' => 'MiniShop3\\Model\\msVendor', 'msCustomer' => 'MiniShop3\\Model\\msCustomer', + 'ms3_customers' => 'MiniShop3\\Model\\msCustomer', 'modUser' => 'MODX\\Revolution\\modUser', 'modUserProfile' => 'MODX\\Revolution\\modUserProfile', 'modResource' => 'MODX\\Revolution\\modResource', + 'site_content' => 'MODX\\Revolution\\modResource', ]; - return $modelMap[$tableName] ?? null; + if (isset($modelMap[$tableName])) { + return $modelMap[$tableName]; + } + + // Fallback: match by xPDO table name for any mapped model class + foreach (array_unique(array_values($modelMap)) as $class) { + $physicalTable = $this->normalizeTableIdentifier($this->modx->getTableName($class)); + if ($physicalTable === $tableName) { + return $class; + } + } + + return null; + } + + /** + * Resolve model class from relation config (supports short names, table names, FQCN). + * + * @param array $relation + */ + protected function resolveModelClassFromRelation(array $relation): ?string + { + $resolvedModelClass = $relation['resolvedModelClass'] ?? null; + if (is_string($resolvedModelClass) && $resolvedModelClass !== '' && class_exists($resolvedModelClass)) { + return $resolvedModelClass; + } + + $table = (string) ($relation['table'] ?? ''); + if ($table !== '' && str_contains($table, '\\') && class_exists($table)) { + return $table; + } + + $modelClass = $this->resolveModelClass($table); + if ($modelClass !== null) { + return $modelClass; + } + + $resolvedTableName = (string) ($relation['resolvedTableName'] ?? ''); + if ($resolvedTableName !== '') { + return $this->resolveModelClass($resolvedTableName); + } + + return null; + } + + /** + * Short table key for SQL JOIN alias (no prefix, no namespace). + * + * @param array $relation + */ + protected function relationTableKey(array $relation): string + { + $table = (string) ($relation['table'] ?? ''); + if (str_contains($table, '\\')) { + $parts = explode('\\', $table); + + return (string) end($parts); + } + + return $this->normalizeTableIdentifier($table); + } + + /** + * Strip table prefix and normalize user input (msVendor, ms3_vendors, modx_ms3_vendors). + */ + protected function normalizeTableIdentifier(string $tableName): string + { + $tableName = trim($tableName); + $prefix = $this->modx->config['table_prefix'] ?? ''; + if ($prefix !== '' && str_starts_with($tableName, $prefix)) { + $tableName = substr($tableName, strlen($prefix)); + } + + return $tableName; } } diff --git a/core/components/minishop3/tests/RelationColumnSpecTest.php b/core/components/minishop3/tests/RelationColumnSpecTest.php new file mode 100644 index 00000000..39e9e412 --- /dev/null +++ b/core/components/minishop3/tests/RelationColumnSpecTest.php @@ -0,0 +1,93 @@ + 'MiniShop3\\Model\\msVendor', + 'foreignKey' => 'vendor_id', + 'alias' => 'rel_msVendor_vendor_id', + 'fields' => [], +]; + +$spec = RelationColumnSpec::fromGroupField($group, [ + 'name' => 'vendor_address', + 'displayField' => 'address', +]); +$assertTrue($spec instanceof RelationColumnSpec, 'valid relation spec is created'); +$assertSame( + '`rel_msVendor_vendor_id`.id = Data.vendor_id', + $spec->joinCondition(), + 'vendor join condition' +); +$assertSame( + '`rel_msVendor_vendor_id`.address AS `vendor_address`', + $spec->selectExpression(), + 'vendor select expression' +); + +$assertNull( + RelationColumnSpec::fromGroupField($group, [ + 'name' => 'article', + 'displayField' => 'address', + ]), + 'reserved field name is rejected' +); + +$assertNull( + RelationColumnSpec::fromGroupField($group, [ + 'name' => 'vendor_address', + 'displayField' => 'address; DROP TABLE', + ]), + 'unsafe displayField is rejected' +); + +$assertTrue( + GridColumnRules::isValidCategoryProductExtraFieldName('vendor_address'), + 'vendor_address is allowed extra field' +); +$assertTrue( + !GridColumnRules::isValidCategoryProductExtraFieldName('vendor_id'), + 'vendor_id is reserved' +); + +fwrite(STDOUT, "OK RelationColumnSpecTest\n"); +exit(0);