mirror of
https://github.com/appwrite/appwrite.git
synced 2026-05-26 13:51:13 +00:00
(refactor): improve advisor module perf, security, and maintainability
- Fix N+1 in Reports/XList (51→4 queries) via skipFilters + batch fetch - Add skipFilters to Reports/Delete and cursor fetch (avoid loading all nested insights/CTAs just for ownership check) - Fix N+1 in deleteReport worker (flat CTA deletion instead of per-insight) - Add advisor entity cleanup on project deletion (reports, insights, CTAs) - Remove resourceInternalId, parentResourceInternalId, $permissions from Insight response model (internal IDs leak DB internals, permissions unused) - Remove dead subQueryInsightCTAs filter registration - Remove stale enum-value comments from platform schema - Fix _key_dismissedAt index to include projectInternalId - Fix scope category from 'Other' to 'Advisor' - Switch action base class from Utopia\Platform\Action to Appwrite\Platform\Action Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
f9ff1166b9
commit
9da4a3260d
@@ -2007,7 +2007,6 @@ $platformCollections = [
|
||||
'filters' => [],
|
||||
],
|
||||
[
|
||||
// Analyzer that produced the report. Possible values: lighthouse, audit, databaseAnalyzer
|
||||
'$id' => ID::custom('type'),
|
||||
'type' => Database::VAR_STRING,
|
||||
'format' => '',
|
||||
@@ -2189,7 +2188,6 @@ $platformCollections = [
|
||||
'filters' => [],
|
||||
],
|
||||
[
|
||||
// Possible values: databaseIndex, databasePerformance, sitePerformance, siteAccessibility, siteSeo, functionPerformance
|
||||
'$id' => ID::custom('type'),
|
||||
'type' => Database::VAR_STRING,
|
||||
'format' => '',
|
||||
@@ -2201,7 +2199,6 @@ $platformCollections = [
|
||||
'filters' => [],
|
||||
],
|
||||
[
|
||||
// Possible values: info, warning, critical
|
||||
'$id' => ID::custom('severity'),
|
||||
'type' => Database::VAR_STRING,
|
||||
'format' => '',
|
||||
@@ -2213,7 +2210,6 @@ $platformCollections = [
|
||||
'filters' => [],
|
||||
],
|
||||
[
|
||||
// Possible values: active, dismissed
|
||||
'$id' => ID::custom('status'),
|
||||
'type' => Database::VAR_STRING,
|
||||
'format' => '',
|
||||
@@ -2225,7 +2221,6 @@ $platformCollections = [
|
||||
'filters' => [],
|
||||
],
|
||||
[
|
||||
// Possible values: databases, collections, sites, functions
|
||||
'$id' => ID::custom('resourceType'),
|
||||
'type' => Database::VAR_STRING,
|
||||
'format' => '',
|
||||
@@ -2417,11 +2412,11 @@ $platformCollections = [
|
||||
'orders' => [],
|
||||
],
|
||||
[
|
||||
'$id' => ID::custom('_key_dismissedAt'),
|
||||
'$id' => ID::custom('_key_project_dismissedAt'),
|
||||
'type' => Database::INDEX_KEY,
|
||||
'attributes' => ['dismissedAt'],
|
||||
'attributes' => ['projectInternalId', 'dismissedAt'],
|
||||
'lengths' => [],
|
||||
'orders' => [Database::ORDER_DESC],
|
||||
'orders' => [Database::ORDER_ASC, Database::ORDER_DESC],
|
||||
],
|
||||
],
|
||||
],
|
||||
|
||||
@@ -365,18 +365,18 @@ return [
|
||||
// Advisor
|
||||
'insights.read' => [
|
||||
'description' => 'Access to read insights under Advisor service.',
|
||||
'category' => 'Other',
|
||||
'category' => 'Advisor',
|
||||
],
|
||||
'insights.write' => [
|
||||
'description' => 'Reserved for Advisor insight ingestion outside CE.',
|
||||
'category' => 'Other',
|
||||
'category' => 'Advisor',
|
||||
],
|
||||
'reports.read' => [
|
||||
'description' => 'Access to read reports under Advisor service.',
|
||||
'category' => 'Other',
|
||||
'category' => 'Advisor',
|
||||
],
|
||||
'reports.write' => [
|
||||
'description' => 'Access to delete reports under Advisor service.',
|
||||
'category' => 'Other',
|
||||
'category' => 'Advisor',
|
||||
],
|
||||
];
|
||||
|
||||
@@ -476,21 +476,6 @@ Database::addFilter(
|
||||
}
|
||||
);
|
||||
|
||||
Database::addFilter(
|
||||
'subQueryInsightCTAs',
|
||||
function (mixed $value) {
|
||||
return;
|
||||
},
|
||||
function (mixed $value, Document $document, Database $database) {
|
||||
return $database->getAuthorization()->skip(fn () => $database
|
||||
->find('insightCTAs', [
|
||||
Query::equal('projectInternalId', [$document->getAttribute('projectInternalId')]),
|
||||
Query::equal('insightInternalId', [$document->getSequence()]),
|
||||
Query::limit(APP_LIMIT_SUBQUERY),
|
||||
]));
|
||||
}
|
||||
);
|
||||
|
||||
Database::addFilter(
|
||||
'subQueryReportInsights',
|
||||
function (mixed $value) {
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
namespace Appwrite\Platform\Modules\Advisor\Http\Insights;
|
||||
|
||||
use Appwrite\Extend\Exception;
|
||||
use Appwrite\Platform\Action;
|
||||
use Appwrite\SDK\AuthType;
|
||||
use Appwrite\SDK\Method;
|
||||
use Appwrite\SDK\Response as SDKResponse;
|
||||
@@ -11,7 +12,6 @@ use Utopia\Database\Database;
|
||||
use Utopia\Database\Document;
|
||||
use Utopia\Database\Query;
|
||||
use Utopia\Database\Validator\UID;
|
||||
use Utopia\Platform\Action;
|
||||
use Utopia\Platform\Scope\HTTP;
|
||||
|
||||
class Get extends Action
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
namespace Appwrite\Platform\Modules\Advisor\Http\Insights;
|
||||
|
||||
use Appwrite\Extend\Exception;
|
||||
use Appwrite\Platform\Action;
|
||||
use Appwrite\SDK\AuthType;
|
||||
use Appwrite\SDK\Method;
|
||||
use Appwrite\SDK\Response as SDKResponse;
|
||||
@@ -15,7 +16,6 @@ use Utopia\Database\Exception\Query as QueryException;
|
||||
use Utopia\Database\Query;
|
||||
use Utopia\Database\Validator\Query\Cursor;
|
||||
use Utopia\Database\Validator\UID;
|
||||
use Utopia\Platform\Action;
|
||||
use Utopia\Platform\Scope\HTTP;
|
||||
use Utopia\Validator\Boolean;
|
||||
|
||||
|
||||
@@ -5,6 +5,7 @@ namespace Appwrite\Platform\Modules\Advisor\Http\Reports;
|
||||
use Appwrite\Event\Delete as DeleteEvent;
|
||||
use Appwrite\Event\Event;
|
||||
use Appwrite\Extend\Exception;
|
||||
use Appwrite\Platform\Action;
|
||||
use Appwrite\SDK\AuthType;
|
||||
use Appwrite\SDK\ContentType;
|
||||
use Appwrite\SDK\Method;
|
||||
@@ -13,7 +14,6 @@ use Appwrite\Utopia\Response;
|
||||
use Utopia\Database\Database;
|
||||
use Utopia\Database\Document;
|
||||
use Utopia\Database\Validator\UID;
|
||||
use Utopia\Platform\Action;
|
||||
use Utopia\Platform\Scope\HTTP;
|
||||
|
||||
class Delete extends Action
|
||||
@@ -71,7 +71,10 @@ class Delete extends Action
|
||||
DeleteEvent $queueForDeletes,
|
||||
Event $queueForEvents
|
||||
): void {
|
||||
$report = $dbForPlatform->getDocument('reports', $reportId);
|
||||
$report = $dbForPlatform->skipFilters(
|
||||
fn () => $dbForPlatform->getDocument('reports', $reportId),
|
||||
['subQueryReportInsights'],
|
||||
);
|
||||
|
||||
if ($report->isEmpty() || $report->getAttribute('projectInternalId') !== $project->getSequence()) {
|
||||
throw new Exception(Exception::REPORT_NOT_FOUND);
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
namespace Appwrite\Platform\Modules\Advisor\Http\Reports;
|
||||
|
||||
use Appwrite\Extend\Exception;
|
||||
use Appwrite\Platform\Action;
|
||||
use Appwrite\SDK\AuthType;
|
||||
use Appwrite\SDK\Method;
|
||||
use Appwrite\SDK\Response as SDKResponse;
|
||||
@@ -10,7 +11,6 @@ use Appwrite\Utopia\Response;
|
||||
use Utopia\Database\Database;
|
||||
use Utopia\Database\Document;
|
||||
use Utopia\Database\Validator\UID;
|
||||
use Utopia\Platform\Action;
|
||||
use Utopia\Platform\Scope\HTTP;
|
||||
|
||||
class Get extends Action
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
namespace Appwrite\Platform\Modules\Advisor\Http\Reports;
|
||||
|
||||
use Appwrite\Extend\Exception;
|
||||
use Appwrite\Platform\Action;
|
||||
use Appwrite\SDK\AuthType;
|
||||
use Appwrite\SDK\Method;
|
||||
use Appwrite\SDK\Response as SDKResponse;
|
||||
@@ -14,7 +15,6 @@ use Utopia\Database\Exception\Order as OrderException;
|
||||
use Utopia\Database\Exception\Query as QueryException;
|
||||
use Utopia\Database\Query;
|
||||
use Utopia\Database\Validator\Query\Cursor;
|
||||
use Utopia\Platform\Action;
|
||||
use Utopia\Platform\Scope\HTTP;
|
||||
use Utopia\Validator\Boolean;
|
||||
|
||||
@@ -82,7 +82,10 @@ class XList extends Action
|
||||
}
|
||||
|
||||
$reportId = $cursor->getValue();
|
||||
$cursorDocument = $dbForPlatform->getDocument('reports', $reportId);
|
||||
$cursorDocument = $dbForPlatform->skipFilters(
|
||||
fn () => $dbForPlatform->getDocument('reports', $reportId),
|
||||
['subQueryReportInsights'],
|
||||
);
|
||||
|
||||
if ($cursorDocument->isEmpty() || $cursorDocument->getAttribute('projectInternalId') !== $project->getSequence()) {
|
||||
throw new Exception(Exception::GENERAL_CURSOR_NOT_FOUND, "Report '{$reportId}' for the 'cursor' value not found.");
|
||||
@@ -94,12 +97,53 @@ class XList extends Action
|
||||
$filterQueries = Query::groupByType($queries)['filters'];
|
||||
|
||||
try {
|
||||
$reports = $dbForPlatform->find('reports', $queries);
|
||||
$reports = $dbForPlatform->skipFilters(
|
||||
fn () => $dbForPlatform->find('reports', $queries),
|
||||
['subQueryReportInsights'],
|
||||
);
|
||||
$total = $includeTotal ? $dbForPlatform->count('reports', $filterQueries, APP_LIMIT_COUNT) : 0;
|
||||
} catch (OrderException $e) {
|
||||
throw new Exception(Exception::DATABASE_QUERY_ORDER_NULL, "The order attribute '{$e->getAttribute()}' had a null value. Cursor pagination requires all documents order attribute values are non-null.");
|
||||
}
|
||||
|
||||
if (!empty($reports)) {
|
||||
$reportSequences = \array_map(fn (Document $r) => $r->getSequence(), $reports);
|
||||
|
||||
$insights = $dbForPlatform->find('insights', [
|
||||
Query::equal('projectInternalId', [$project->getSequence()]),
|
||||
Query::equal('reportInternalId', $reportSequences),
|
||||
Query::limit(APP_LIMIT_SUBQUERY),
|
||||
]);
|
||||
|
||||
if (!empty($insights)) {
|
||||
$insightSequences = \array_map(fn (Document $i) => $i->getSequence(), $insights);
|
||||
|
||||
$ctas = $dbForPlatform->find('insightCTAs', [
|
||||
Query::equal('projectInternalId', [$project->getSequence()]),
|
||||
Query::equal('insightInternalId', $insightSequences),
|
||||
Query::limit(APP_LIMIT_SUBQUERY),
|
||||
]);
|
||||
|
||||
$ctasByInsight = [];
|
||||
foreach ($ctas as $cta) {
|
||||
$ctasByInsight[$cta->getAttribute('insightInternalId')][] = $cta;
|
||||
}
|
||||
|
||||
foreach ($insights as $insight) {
|
||||
$insight->setAttribute('ctas', $ctasByInsight[$insight->getSequence()] ?? []);
|
||||
}
|
||||
}
|
||||
|
||||
$insightsByReport = [];
|
||||
foreach ($insights as $insight) {
|
||||
$insightsByReport[$insight->getAttribute('reportInternalId')][] = $insight;
|
||||
}
|
||||
|
||||
foreach ($reports as $report) {
|
||||
$report->setAttribute('insights', $insightsByReport[$report->getSequence()] ?? []);
|
||||
}
|
||||
}
|
||||
|
||||
$response->dynamic(new Document([
|
||||
'reports' => $reports,
|
||||
'total' => $total,
|
||||
|
||||
@@ -229,15 +229,25 @@ class Deletes extends Action
|
||||
$projectInternalId = $project->getSequence();
|
||||
$reportInternalId = $report->getSequence();
|
||||
|
||||
$insights = $dbForPlatform->find('insights', [
|
||||
Query::equal('projectInternalId', [$projectInternalId]),
|
||||
Query::equal('reportInternalId', [$reportInternalId]),
|
||||
Query::limit(APP_LIMIT_SUBQUERY),
|
||||
]);
|
||||
|
||||
if (!empty($insights)) {
|
||||
$insightSequences = \array_map(fn (Document $i) => $i->getSequence(), $insights);
|
||||
|
||||
$this->deleteByGroup('insightCTAs', [
|
||||
Query::equal('projectInternalId', [$projectInternalId]),
|
||||
Query::equal('insightInternalId', $insightSequences),
|
||||
], $dbForPlatform);
|
||||
}
|
||||
|
||||
$this->deleteByGroup('insights', [
|
||||
Query::equal('projectInternalId', [$projectInternalId]),
|
||||
Query::equal('reportInternalId', [$reportInternalId]),
|
||||
], $dbForPlatform, function (Document $insight) use ($dbForPlatform, $projectInternalId) {
|
||||
$this->deleteByGroup('insightCTAs', [
|
||||
Query::equal('projectInternalId', [$projectInternalId]),
|
||||
Query::equal('insightInternalId', [$insight->getSequence()]),
|
||||
], $dbForPlatform);
|
||||
});
|
||||
], $dbForPlatform);
|
||||
}
|
||||
|
||||
private function cleanDatabase(
|
||||
@@ -735,6 +745,36 @@ class Deletes extends Action
|
||||
Console::error('Failed to delete schedules: ' . $th->getMessage());
|
||||
}
|
||||
|
||||
// Delete Advisor CTAs
|
||||
try {
|
||||
$this->deleteByGroup('insightCTAs', [
|
||||
Query::equal('projectInternalId', [$projectInternalId]),
|
||||
Query::orderAsc()
|
||||
], $dbForPlatform);
|
||||
} catch (Throwable $th) {
|
||||
Console::error('Failed to delete insight CTAs: ' . $th->getMessage());
|
||||
}
|
||||
|
||||
// Delete Advisor insights
|
||||
try {
|
||||
$this->deleteByGroup('insights', [
|
||||
Query::equal('projectInternalId', [$projectInternalId]),
|
||||
Query::orderAsc()
|
||||
], $dbForPlatform);
|
||||
} catch (Throwable $th) {
|
||||
Console::error('Failed to delete insights: ' . $th->getMessage());
|
||||
}
|
||||
|
||||
// Delete Advisor reports
|
||||
try {
|
||||
$this->deleteByGroup('reports', [
|
||||
Query::equal('projectInternalId', [$projectInternalId]),
|
||||
Query::orderAsc()
|
||||
], $dbForPlatform);
|
||||
} catch (Throwable $th) {
|
||||
Console::error('Failed to delete reports: ' . $th->getMessage());
|
||||
}
|
||||
|
||||
/**
|
||||
* @var Database $dbForProject
|
||||
*/
|
||||
|
||||
@@ -28,13 +28,6 @@ class Insight extends Model
|
||||
'default' => '',
|
||||
'example' => self::TYPE_DATETIME_EXAMPLE,
|
||||
])
|
||||
->addRule('$permissions', [
|
||||
'type' => self::TYPE_STRING,
|
||||
'description' => 'Insight permissions. [Learn more about permissions](https://appwrite.io/docs/permissions).',
|
||||
'default' => [],
|
||||
'example' => ['read("any")'],
|
||||
'array' => true,
|
||||
])
|
||||
->addRule('reportId', [
|
||||
'type' => self::TYPE_STRING,
|
||||
'description' => 'Parent report ID. Insights always belong to a report.',
|
||||
@@ -71,12 +64,6 @@ class Insight extends Model
|
||||
'default' => '',
|
||||
'example' => 'main',
|
||||
])
|
||||
->addRule('resourceInternalId', [
|
||||
'type' => self::TYPE_STRING,
|
||||
'description' => 'Internal ID of the resource the insight is about.',
|
||||
'default' => '',
|
||||
'example' => '5e5ea5c16897e',
|
||||
])
|
||||
->addRule('parentResourceType', [
|
||||
'type' => self::TYPE_STRING,
|
||||
'description' => 'Plural noun for the parent resource that contains the insight\'s resource, e.g. an insight about a column index on a table → resourceType=indexes, parentResourceType=tables. Empty when the resource has no parent.',
|
||||
@@ -89,12 +76,6 @@ class Insight extends Model
|
||||
'default' => '',
|
||||
'example' => 'orders',
|
||||
])
|
||||
->addRule('parentResourceInternalId', [
|
||||
'type' => self::TYPE_STRING,
|
||||
'description' => 'Internal ID of the parent resource. Empty when the resource has no parent.',
|
||||
'default' => '',
|
||||
'example' => '5e5ea5c16897e',
|
||||
])
|
||||
->addRule('title', [
|
||||
'type' => self::TYPE_STRING,
|
||||
'description' => 'Insight title.',
|
||||
|
||||
Reference in New Issue
Block a user