ronantakizawa/codereview-bench huggingface dataset:
Repository: appwrite/appwrite
File: app/controllers/api/console.php (PHP)

Code before:
        curl_setopt($ch, CURLOPT_HEADERFUNCTION, function ($curl, $header) use (&$responseHeaders) {
            $len = strlen($header);
            $header = explode(':', $header, 2);

            if (count($header) < 2) { // ignore invalid headers
                return $len;
            }

            $responseHeaders[strtolower(trim($header[0]))] = trim($header[1]);

            return $len;
        });

        curl_setopt($ch, CURLOPT_POSTFIELDS, $query);

        curl_exec($ch);

        curl_close($ch);

        $response->chunk('', true);
    });

App::get('v1/console/resources/:resourceId')
    ->desc('Check resource ID availability')
    ->groups(['api', 'projects'])
    ->label('scope', 'projects.read')
    ->label('sdk.auth', [APP_AUTH_TYPE_ADMIN])
    ->label('sdk.namespace', 'console')
    ->label('sdk.method', 'checkResourceAvailability')
    ->label('sdk.response.code', Response::STATUS_CODE_OK)
    ->label('sdk.response.type', Response::CONTENT_TYPE_JSON)
    ->label('sdk.response.model', Response::MODEL_NONE)
    ->label('abuse-limit', 10)
    ->label('abuse-key', 'userId:{userId}')
    ->param('resourceId', '', new UID(), 'ID of the resource.')
    ->param('type', '', new WhiteList(['rules']), 'Resource type.')
    ->inject('response')
    ->inject('dbForConsole')
    ->action(function (string $resourceId, string $type, Response $response, Database $dbForConsole) {
        $document = Authorization::skip(fn () => $dbForConsole->getDocument('rules', $resourceId));

        if (!$document->isEmpty()) {
            throw new Exception(Exception::RESOURCE_ALREADY_EXISTS, 'Resource ID is already in use.');
        }

        $response->noContent();
    });


Review comment (suggestion):
Lets check with Christy scope for this endpoint. Maybe `rules.write` instead? (because we do it in that flow); or maybe none at all

Code after:
        $handleEvent = function ($ch, $data) use ($response) {
            $response->chunk($data);

            return \strlen($data);
        };

        curl_setopt($ch, CURLOPT_WRITEFUNCTION, $handleEvent);

        curl_setopt($ch, CURLOPT_CUSTOMREQUEST, 'POST');
        curl_setopt($ch, CURLOPT_FOLLOWLOCATION, true);
        curl_setopt($ch, CURLOPT_HTTPHEADER, $headers);
        curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, 0);
        curl_setopt($ch, CURLOPT_TIMEOUT, 9000);
        curl_setopt($ch, CURLOPT_HEADERFUNCTION, function ($curl, $header) use (&$responseHeaders) {
            $len = strlen($header);
            $header = explode(':', $header, 2);

            if (count($header) < 2) { // ignore invalid headers
                return $len;
            }

            $responseHeaders[strtolower(trim($header[0]))] = trim($header[1]);

            return $len;
        });

        curl_setopt($ch, CURLOPT_POSTFIELDS, $query);

        curl_exec($ch);

        curl_close($ch);

        $response->chunk('', true);
    });
Repository: appwrite/appwrite
File: src/Appwrite/Platform/Modules/Console/Http/Resources/Get.php (PHP)

Code before:
            ->inject('dbForPlatform')
            ->callback([$this, 'action']);
    }

    public function action(string $value, string $type, Response $response, Database $dbForPlatform)
    {
        if ($type == 'rules') {
            $validator = new Domain($value);

            if (!$validator->isValid($value)) {
                throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, $validator->getDescription());
            }

            $document = Authorization::skip(fn () => $dbForPlatform->findOne('rules', [
                Query::equal('domain', [$value]),
            ]));

            if (!$document->isEmpty()) {
                throw new Exception(Exception::RESOURCE_ALREADY_EXISTS);
            }

            $response->noContent();
        }

        throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'Invalid type');
    }
}


Review comment (suggestion):
```suggestion
	// Only occurs if type is added into whitelist, but not supported in action
        throw new Exception(Exception::GENERAL_ARGUMENT_INVALID);
```

Code after:
            ->inject('dbForPlatform')
            ->callback([$this, 'action']);
    }

    public function action(string $value, string $type, Response $response, Database $dbForPlatform)
    {
        if ($type === 'rules') {
            $validator = new Domain($value);

            if (!$validator->isValid($value)) {
                throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, $validator->getDescription());
            }

            $document = Authorization::skip(fn () => $dbForPlatform->findOne('rules', [
                Query::equal('domain', [$value]),
            ]));

            if (!$document->isEmpty()) {
                throw new Exception(Exception::RESOURCE_ALREADY_EXISTS);
            }

            $response->noContent();
        }

        // Only occurs if type is added into whitelist, but not supported in action
        throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'Invalid type');
    }
}
Repository: appwrite/appwrite
File: tests/e2e/Services/Functions/FunctionsConsoleClientTest.php (PHP)

Code before:
        $variable = $this->createVariable($functionId, [
            'key' => 'CUSTOM_VARIABLE',
            'value' => 'test',
            'secret' => true,
        ]);

        $this->assertEquals(201, $variable['headers']['status-code']);
        $this->assertNotEmpty($variable['body']['$id']);
        $this->assertEquals('CUSTOM_VARIABLE', $variable['body']['key']);
        $this->assertEquals('', $variable['body']['value']);
        $this->assertEquals(true, $variable['body']['secret']);

        $deploymentId = $this->setupDeployment($functionId, [
            'entrypoint' => 'index.js',
            'code' => $this->packageFunction('node'),
            'activate' => true
        ]);

        $this->assertNotEmpty($deploymentId);

        $execution = $this->createExecution($functionId);

        $this->assertEquals(201, $execution['headers']['status-code']);
        $this->assertEmpty($execution['body']['logs']);
        $this->assertEmpty($execution['body']['errors']);
        $this->assertStringContainsString('"CUSTOM_VARIABLE":"test"', $execution['body']['responseBody']);

        $this->cleanupFunction($functionId);
    }
}


Review comment (security):
Instead of this, since response is JSON, let's do something like

```
$body = json_decode(execution.body.responseBody)
```
So then the test can be along lines
```
assertEquals('aSecretValue', $body['CUSTOM_VARIABLE'])
```

Code after:
        $variable = $this->createVariable($functionId, [
            'key' => 'CUSTOM_VARIABLE',
            'value' => 'a_secret_value',
            'secret' => true,
        ]);

        $this->assertEquals(201, $variable['headers']['status-code']);
        $this->assertNotEmpty($variable['body']['$id']);
        $this->assertEquals('CUSTOM_VARIABLE', $variable['body']['key']);
        $this->assertEquals('', $variable['body']['value']);
        $this->assertEquals(true, $variable['body']['secret']);

        $deploymentId = $this->setupDeployment($functionId, [
            'entrypoint' => 'index.js',
            'code' => $this->packageFunction('node'),
            'activate' => true
        ]);

        $this->assertNotEmpty($deploymentId);

        $execution = $this->createExecution($functionId);

        $this->assertEquals(201, $execution['headers']['status-code']);
        $this->assertEmpty($execution['body']['logs']);
        $this->assertEmpty($execution['body']['errors']);
        $body = json_decode($execution['body']['responseBody']);
        $this->assertEquals('a_secret_value', $body->CUSTOM_VARIABLE);

        $this->cleanupFunction($functionId);
    }
}
Repository: appwrite/appwrite
File: src/Appwrite/Migration/Version/V20.php (PHP)

Code before:
        $this->createInfMetric('sessions', $value);
    }

    /**
     * @param string $metric
     * @param int $value
     * @return void
     * @throws Exception
     * @throws Authorization
     * @throws Structure
     */
    protected function createInfMetric(string $metric, int $value): void
    {
        try {
            /**
             * Creating inf metric
             */
            Console::log("Creating inf metric to {$metric}");
            $id = \md5("_inf_{$metric}");
            $this->projectDB->createDocument('stats', new Document([
                '$id' => $id,
                'metric' => $metric,
                'period' => 'inf',
                'value' => $value,
                'time' => null,
                'region' => 'fra',
            ]));
        } catch (Duplicate $th) {
            Console::warning("Error while creating inf metric: duplicate id {$metric} {$id}");
        }
    }

    /**
     * @param string $from
     * @param string $to
     * @return void
     * @throws Exception
     */
    protected function migrateUsageMetrics(string $from, string $to): void
    {
        /**
         * inf metric
         */
        if (
            str_contains($from, '$all') ||
            str_contains($from, '.total')
        ) {
            $query = $this->projectDB->sum('stats', 'value', [
                Query::equal('metric', [$from]),
                Query::equal('period', ['1d']),
            ]);

Review comment (suggestion):
```suggestion
                'region' => 'default',
```

Code after:
        $this->createInfMetric('sessions', $value);
    }

    /**
     * @param string $metric
     * @param int $value
     * @return void
     * @throws Exception
     * @throws Authorization
     * @throws Structure
     */
    protected function createInfMetric(string $metric, int $value): void
    {
        try {
            /**
             * Creating inf metric
             */
            Console::log("Creating inf metric to {$metric}");
            $id = \md5("_inf_{$metric}");
            $this->projectDB->createDocument('stats', new Document([
                '$id' => $id,
                'metric' => $metric,
                'period' => 'inf',
                'value' => $value,
                'time' => null,
                'region' => 'default',
            ]));
        } catch (Duplicate $th) {
            Console::warning("Error while creating inf metric: duplicate id {$metric} {$id}");
        }
    }

    /**
     * @param string $from
     * @param string $to
     * @return void
     * @throws Exception
     */
    protected function migrateUsageMetrics(string $from, string $to): void
    {
        /**
         * inf metric
         */
        if (
            str_contains($from, '$all') ||
            str_contains($from, '.total')
        ) {
            $query = $this->projectDB->sum('stats', 'value', [
                Query::equal('metric', [$from]),
                Query::equal('period', ['1d']),
            ]);
Repository: appwrite/appwrite
File: tests/e2e/Services/Sites/SitesCustomServerTest.php (PHP)

Code before:
        ]);
        $this->assertNotEmpty($siteId);

        $site = $this->getSite($siteId);
        $this->assertEquals('200', $site['headers']['status-code']);
        $this->assertArrayHasKey('adapter', $site['body']);

        $domain = $this->setupSiteDomain($siteId);
        $this->assertNotEmpty($domain);

        $deploymentId = $this->setupDeployment($siteId, [
            'code' => $this->packageSite('subProjects'),
            'activate' => 'true'
        ]);
        $this->assertNotEmpty($deploymentId);

        $site = $this->getSite($siteId);

        $proxyClient = new Client();
        $proxyClient->setEndpoint('http://' . $domain);
        $response = $proxyClient->call(Client::METHOD_GET, '/');
        $this->assertEquals(200, $response['headers']['status-code']);
        $response1 = $proxyClient->call(Client::METHOD_GET, '/project1');
        $this->assertEquals(200, $response1['headers']['status-code']);
        $response2 = $proxyClient->call(Client::METHOD_GET, '/project1/');
        $this->assertEquals(200, $response2['headers']['status-code']);
        $this->cleanupSite($siteId);
    }
}


Review comment (suggestion):
Let's also ensure that body includes proper content, `Sub-directory index`, `Sub-directory project1`

Code after:
        ]);

        $this->assertNotEmpty($siteId);

        $domain = $this->setupSiteDomain($siteId);
        $this->assertNotEmpty($domain);
        $proxyClient = new Client();
        $proxyClient->setEndpoint('http://' . $domain);

        $deployment = $this->createDeployment($siteId, [
            'code' => $this->packageSite('astro'),
            'activate' => true
        ]);
        $this->assertEquals(202, $deployment['headers']['status-code']);

        $deploymentId = $deployment['body']['$id'];
        $this->assertNotEmpty($deploymentId);

        $this->assertEventually(function () use ($siteId, $deploymentId) {
            $deployment = $this->getDeployment($siteId, $deploymentId);
            $this->assertEquals('failed', $deployment['body']['status'], json_encode($deployment['body'], JSON_PRETTY_PRINT));
        }, 100000, 500);

        $response = $proxyClient->call(Client::METHOD_GET, '/');
        $this->assertStringContainsString('build_failed', $response['body']);

        $this->cleanupSite($siteId);
    }

    public function testPermanentRedirect(): void
    {
        $siteId = $this->setupSite([
            'siteId' => ID::unique(),
            'name' => 'Sub project site',
            'framework' => 'other',
            'buildRuntime' => 'node-22',
            'outputDirectory' => './'
        ]);
        $this->assertNotEmpty($siteId);

        $domain = $this->setupSiteDomain($siteId);
        $this->assertNotEmpty($domain);

        $deploymentId = $this->setupDeployment($siteId, [
            'code' => $this->packageSite('sub-directories'),
            'activate' => 'true'
        ]);
        $this->assertNotEmpty($deploymentId);

        $proxyClient = new Client();
        $proxyClient->setEndpoint('http://' . $domain);
Repository: appwrite/appwrite
File: src/Appwrite/Utopia/Database/Validator/Queries/Base.php (PHP)

Code before:
            'type' => Database::VAR_STRING,
            'array' => false,
        ]);
        $attributes[] = new Document([
            'key' => '$createdAt',
            'type' => Database::VAR_DATETIME,
            'array' => false,
        ]);
        $attributes[] = new Document([
            'key' => '$updatedAt',
            'type' => Database::VAR_DATETIME,
            'array' => false,
        ]);

        $internalId = new Document([
            'key' => '$internalId',
            'type' => Database::VAR_STRING,
            'array' => false,
        ]);

        $validators = [
            new Limit(),
            new Offset(),
            new Cursor(),
            new Filter($attributes, APP_DATABASE_QUERY_MAX_VALUES),
            new Order(array_merge($attributes, [$internalId])),
        ];

        parent::__construct($validators);
    }
}


Review comment (suggestion):
```suggestion
            new Order([...$attributes, $internalId])),
```

Code after:
            'type' => Database::VAR_STRING,
            'array' => false,
        ]);
        $attributes[] = new Document([
            'key' => '$createdAt',
            'type' => Database::VAR_DATETIME,
            'array' => false,
        ]);
        $attributes[] = new Document([
            'key' => '$updatedAt',
            'type' => Database::VAR_DATETIME,
            'array' => false,
        ]);

        $internalId = new Document([
            'key' => '$internalId',
            'type' => Database::VAR_STRING,
            'array' => false,
        ]);

        $validators = [
            new Limit(),
            new Offset(),
            new Cursor(),
            new Filter($attributes, APP_DATABASE_QUERY_MAX_VALUES),
            new Order([...$attributes, $internalId]),
        ];

        parent::__construct($validators);
    }
}
Repository: appwrite/appwrite
File: src/Appwrite/Platform/Workers/Messaging.php (PHP)

Code before:
                ],
                'textmagic' => [
                    'username' => $user,
                    'apiKey' => $password
                ],
                'telesign' => [
                    'customerId' => $user,
                    'apiKey' => $password
                ],
                'msg91' => [
                    'senderId' => $user,
                    'authKey' => $password,
                    'templateId' => $dsn->getParam('templateId', $from),
                ],
                'vonage' => [
                    'apiKey' => $user,
                    'apiSecret' => $password
                ],
                'fast2sms' => [
                    'senderId' => $user,
                    'apiKey' => $password,
                    'messageId' => $dsn->getParam('messageId'),
                    'useDLT' => $dsn->getParam('useDLT'),
                ],
                'inforu' => [
                    'senderId' => $dsn->getParam('senderId', $from),
                    'apiKey' => $password,
                ],
                default => null
            },
            'options' => match ($host) {
                'twilio' => [
                    'from' => \str_starts_with($from, 'MG') ? null : $from
                ],
                default => [
                    'from' => $from
                ]
            }
        ]);

        return $provider;
    }
}


Review comment (suggestion):
```suggestion
                    'senderId' => $user,
```
we can just pass the senderID as the user in the DSN right? 

`sms://senderId:password@inforu` 

Code after:
                ],
                'textmagic' => [
                    'username' => $user,
                    'apiKey' => $password
                ],
                'telesign' => [
                    'customerId' => $user,
                    'apiKey' => $password
                ],
                'msg91' => [
                    'senderId' => $user,
                    'authKey' => $password,
                    'templateId' => $dsn->getParam('templateId', $from),
                ],
                'vonage' => [
                    'apiKey' => $user,
                    'apiSecret' => $password
                ],
                'fast2sms' => [
                    'senderId' => $user,
                    'apiKey' => $password,
                    'messageId' => $dsn->getParam('messageId'),
                    'useDLT' => $dsn->getParam('useDLT'),
                ],
                'inforu' => [
                    'senderId' => $user,
                    'apiKey' => $password,
                ],
                default => null
            },
            'options' => match ($host) {
                'twilio' => [
                    'from' => \str_starts_with($from, 'MG') ? null : $from
                ],
                default => [
                    'from' => $from
                ]
            }
        ]);

        return $provider;
    }
}
Repository: appwrite/appwrite
File: src/Appwrite/Utopia/Response/Model/AttributeString.php (PHP)

Code before:

use Appwrite\Utopia\Response;

class AttributeString extends Attribute
{
    public function __construct()
    {
        parent::__construct();

        $this
            ->addRule('size', [
                'type' => self::TYPE_INTEGER,
                'description' => 'Attribute size.',
                'default' => 0,
                'example' => 128,
            ])
            ->addRule('default', [
                'type' => self::TYPE_STRING,
                'description' => 'Default value for attribute when not provided. Cannot be set when attribute is required.',
                'default' => null,
                'required' => false,
                'example' => 'default',
            ])
            ->addRule('encrypt', [
                'type' => self::TYPE_BOOLEAN,
                'description' => 'Encryption status for the string attribute. Encryption enhances security by not storing any plain text values in the database. However, encrypted attributes cannot be queried.',
                'default' => false,
                'required' => false,
                'example' => false,
            ])
        ;
    }

    public array $conditions = [
        'type' => self::TYPE_STRING,
    ];

    /**
     * Get Name
     *
     * @return string
     */
    public function getName(): string
    {
        return 'AttributeString';
    }

    /**
     * Get Type
     *
     * @return string

Review comment (suggestion):
```suggestion
                'description' => 'Defines whether this attribute is encrypted or not.
```

Code after:

use Appwrite\Utopia\Response;

class AttributeString extends Attribute
{
    public function __construct()
    {
        parent::__construct();

        $this
            ->addRule('size', [
                'type' => self::TYPE_INTEGER,
                'description' => 'Attribute size.',
                'default' => 0,
                'example' => 128,
            ])
            ->addRule('default', [
                'type' => self::TYPE_STRING,
                'description' => 'Default value for attribute when not provided. Cannot be set when attribute is required.',
                'default' => null,
                'required' => false,
                'example' => 'default',
            ])
            ->addRule('encrypt', [
                'type' => self::TYPE_BOOLEAN,
                'description' => 'Defines whether this attribute is encrypted or not.',
                'default' => false,
                'required' => false,
                'example' => false,
            ])
        ;
    }

    public array $conditions = [
        'type' => self::TYPE_STRING,
    ];

    /**
     * Get Name
     *
     * @return string
     */
    public function getName(): string
    {
        return 'AttributeString';
    }

    /**
     * Get Type
     *
     * @return string
Repository: appwrite/appwrite
File: app/config/templates/site.php (PHP)

Code before:
<?php

use Utopia\System\System;

/**
 * List of Appwrite Sites templates
 */

$protocol = System::getEnv('_APP_OPTIONS_FORCE_HTTPS') == 'disabled' ? 'http' : 'https';
$hostname = System::getEnv('_APP_CONSOLE_DOMAIN', System::getEnv('_APP_DOMAIN', ''));

// TODO: Development override
if (System::getEnv('_APP_ENV') === 'development') {
    $hostname = 'localhost';
}

$url = $protocol . '://' . $hostname;

class UseCases
{
    public const PORTFOLIO = 'portfolio';
    public const STARTER = 'starter';
    public const EVENTS = 'events';
    public const ECOMMERCE = 'ecommerce';
    public const DOCUMENTATION = 'documentation';
    public const BLOG = 'blog';
}

const TEMPLATE_FRAMEWORKS = [
    'SVELTEKIT' => [
        'key' => 'sveltekit',
        'name' => 'SvelteKit',
        'installCommand' => 'npm install',
        'buildCommand' => 'npm run build',
        'outputDirectory' => './build',

Review comment (suggestion):
With this, let's set _APP_CONSOLE_DOMAIN=localhost in .env for local developemt, and tests.
Then, we can remove development override just below, and it should still work. Please test, by visitng Console and making sure templates have screenshots properly

Code after:
<?php

use Utopia\System\System;

/**
 * List of Appwrite Sites templates
 */

$protocol = System::getEnv('_APP_OPTIONS_FORCE_HTTPS') === 'disabled' ? 'http' : 'https';
$hostname = System::getEnv('_APP_CONSOLE_DOMAIN', System::getEnv('_APP_DOMAIN', ''));

$url = $protocol . '://' . $hostname;

class UseCases
{
    public const PORTFOLIO = 'portfolio';
    public const STARTER = 'starter';
    public const EVENTS = 'events';
    public const ECOMMERCE = 'ecommerce';
    public const DOCUMENTATION = 'documentation';
    public const BLOG = 'blog';
}

const TEMPLATE_FRAMEWORKS = [
    'SVELTEKIT' => [
        'key' => 'sveltekit',
        'name' => 'SvelteKit',
        'installCommand' => 'npm install',
        'buildCommand' => 'npm run build',
        'outputDirectory' => './build',
        'buildRuntime' => 'node-22',
        'adapter' => 'ssr',
        'fallbackFile' => '',
    ],
    'NEXTJS' => [