Skip to content

Commit ca2ae11

Browse files
lukasdotcomlukasdotcom
committed
Implement feedback
Signed-off-by: Lukas Schaefer <[email protected]>
1 parent 1509bca commit ca2ae11

15 files changed

+103
-99
lines changed

‎appinfo/info.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ Negative:
101101
102102
Learn more about the Nextcloud Ethical AI Rating [in our blog](https://nextcloud.com/blog/nextcloud-ethical-ai-rating/).
103103
]]> </description>
104-
<version>3.7.1</version>
104+
<version>3.8.0</version>
105105
<licence>agpl</licence>
106106
<author>Julien Veyssier</author>
107107
<namespace>OpenAi</namespace>

‎lib/Db/QuotaRule.php

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
use JsonSerializable;
1313
use OCP\AppFramework\Db\Entity;
14+
use OCP\DB\Types;
1415
use ReturnTypeWillChange;
1516

1617
/**
@@ -34,20 +35,20 @@ class QuotaRule extends Entity implements JsonSerializable {
3435
protected $pool;
3536

3637
public function __construct() {
37-
$this->addType('type', 'integer');
38-
$this->addType('amount', 'integer');
39-
$this->addType('priority', 'integer');
40-
$this->addType('pool', 'boolean');
38+
$this->addType('type', Types::INTEGER);
39+
$this->addType('amount', Types::INTEGER);
40+
$this->addType('priority', Types::INTEGER);
41+
$this->addType('pool', Types::BOOLEAN);
4142
}
4243

4344
#[ReturnTypeWillChange]
4445
public function jsonSerialize() {
4546
return [
46-
'id' => $this->id,
47-
'type' => $this->type,
48-
'amount' => $this->amount,
49-
'priority' => $this->priority,
50-
'pool' => $this->pool
47+
'id' => $this->getId(),
48+
'type' => $this->getType(),
49+
'amount' => $this->getAmount(),
50+
'priority' => $this->getPriority(),
51+
'pool' => $this->getPool()
5152
];
5253
}
5354
}

‎lib/Db/QuotaRuleMapper.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
class QuotaRuleMapper extends QBMapper {
2121
public function __construct(
2222
IDBConnection $db,
23-
private QuotaUserMapper $quotaUserMapper,
2423
) {
2524
parent::__construct($db, 'openai_quota_rule', QuotaRule::class);
2625
}
@@ -54,7 +53,7 @@ public function getRule(int $quotaType, string $userId, array $groups): QuotaRul
5453
->from($this->getTableName(), 'r')
5554
->leftJoin('r', 'openai_quota_user', 'u', 'r.id = u.rule_id')
5655
->where(
57-
$qb->expr()->eq('type', $qb->createNamedParameter($quotaType, IQueryBuilder::PARAM_INT))
56+
$qb->expr()->eq('r.type', $qb->createNamedParameter($quotaType, IQueryBuilder::PARAM_INT))
5857
)->andWhere(
5958
$qb->expr()->orX(
6059
$qb->expr()->andX(

‎lib/Db/QuotaUsage.php

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
use JsonSerializable;
1313
use OCP\AppFramework\Db\Entity;
14+
use OCP\DB\Types;
1415
use ReturnTypeWillChange;
1516

1617
/**
@@ -38,22 +39,22 @@ class QuotaUsage extends Entity implements JsonSerializable {
3839
protected $pool;
3940

4041
public function __construct() {
41-
$this->addType('user_id', 'string');
42-
$this->addType('type', 'integer');
43-
$this->addType('units', 'integer');
44-
$this->addType('timestamp', 'integer');
45-
$this->addType('pool', 'integer');
42+
$this->addType('user_id', Types::STRING);
43+
$this->addType('type', Types::INTEGER);
44+
$this->addType('units', Types::INTEGER);
45+
$this->addType('timestamp', Types::INTEGER);
46+
$this->addType('pool', Types::INTEGER);
4647
}
4748

4849
#[ReturnTypeWillChange]
4950
public function jsonSerialize() {
5051
return [
51-
'id' => $this->id,
52-
'user_id' => $this->userId,
53-
'type' => $this->type,
54-
'units' => $this->units,
55-
'timestamp' => $this->timestamp,
56-
'pool' => $this->pool
52+
'id' => $this->getId(),
53+
'user_id' => $this->getUserId(),
54+
'type' => $this->getType(),
55+
'units' => $this->getUnits(),
56+
'timestamp' => $this->getTimestamp(),
57+
'pool' => $this->getPool()
5758
];
5859
}
5960
}

‎lib/Db/QuotaUsageMapper.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ public function getQuotaUnitsOfUser(string $userId, int $type): int {
193193
* @return QuotaUsage
194194
* @throws Exception
195195
*/
196-
public function createQuotaUsage(string $userId, int $type, int $units, ?int $pool = -1): QuotaUsage {
196+
public function createQuotaUsage(string $userId, int $type, int $units, int $pool = -1): QuotaUsage {
197197

198198
$quotaUsage = new QuotaUsage();
199199
$quotaUsage->setUserId($userId);

‎lib/Db/QuotaUser.php

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
use JsonSerializable;
1111
use OCP\AppFramework\Db\Entity;
12+
use OCP\DB\Types;
1213
use ReturnTypeWillChange;
1314

1415
/**
@@ -28,18 +29,18 @@ class QuotaUser extends Entity implements JsonSerializable {
2829
protected $entityId;
2930

3031
public function __construct() {
31-
$this->addType('rule_id', 'integer');
32-
$this->addType('entity_type', 'string');
33-
$this->addType('entity_id', 'string');
32+
$this->addType('rule_id', Types::INTEGER);
33+
$this->addType('entity_type', Types::STRING);
34+
$this->addType('entity_id', Types::STRING);
3435
}
3536

3637
#[ReturnTypeWillChange]
3738
public function jsonSerialize() {
3839
return [
39-
'id' => $this->id,
40-
'rule_id' => $this->ruleId,
41-
'entity_type' => $this->entityType,
42-
'entity_id' => $this->entityId
40+
'id' => $this->getId(),
41+
'rule_id' => $this->getRuleId(),
42+
'entity_type' => $this->getEntityType(),
43+
'entity_id' => $this->getEntityId()
4344
];
4445
}
4546
}

‎lib/Db/QuotaUserMapper.php

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -43,28 +43,35 @@ public function setUsers(int $ruleId, array $users): void {
4343
$qb->select('*')
4444
->from($this->getTableName())
4545
->where($qb->expr()->eq('rule_id', $qb->createNamedParameter($ruleId, IQueryBuilder::PARAM_INT)));
46-
$oldUsers = $this->findEntities($qb);
47-
$oldUsersById = array_reduce($oldUsers, function (array $carry, QuotaUser $oldUser) {
48-
$carry[$oldUser->getEntityType() . '-' . $oldUser->getEntityId()] = $oldUser;
49-
return $carry;
50-
}, []);
51-
$usersById = [];
52-
// Add users that are in the new list but not in the old list
53-
foreach ($users as $user) {
54-
if (!isset($oldUsersById[$user['entity_type'] . '-' . $user['entity_id']])) {
55-
$newUser = new QuotaUser();
56-
$newUser->setRuleId($ruleId);
57-
$newUser->setEntityType($user['entity_type']);
58-
$newUser->setEntityId($user['entity_id']);
59-
$this->insert($newUser);
46+
$this->db->beginTransaction();
47+
try {
48+
$oldUsers = $this->findEntities($qb);
49+
$oldUsersById = array_reduce($oldUsers, function (array $carry, QuotaUser $oldUser) {
50+
$carry[$oldUser->getEntityType() . '-' . $oldUser->getEntityId()] = $oldUser;
51+
return $carry;
52+
}, []);
53+
$usersById = [];
54+
// Add users that are in the new list but not in the old list
55+
foreach ($users as $user) {
56+
if (!isset($oldUsersById[$user['entity_type'] . '-' . $user['entity_id']])) {
57+
$newUser = new QuotaUser();
58+
$newUser->setRuleId($ruleId);
59+
$newUser->setEntityType($user['entity_type']);
60+
$newUser->setEntityId($user['entity_id']);
61+
$this->insert($newUser);
62+
}
63+
$usersById[$user['entity_type'] . '-' . $user['entity_id']] = $user;
6064
}
61-
$usersById[$user['entity_type'] . '-' . $user['entity_id']] = $user;
62-
}
63-
// Delete users that are not in the new list but are in the old list
64-
foreach ($oldUsers as $oldUser) {
65-
if (!isset($usersById[$oldUser->getEntityType() . '-' . $oldUser->getEntityId()])) {
66-
$this->delete($oldUser);
65+
// Delete users that are not in the new list but are in the old list
66+
foreach ($oldUsers as $oldUser) {
67+
if (!isset($usersById[$oldUser->getEntityType() . '-' . $oldUser->getEntityId()])) {
68+
$this->delete($oldUser);
69+
}
6770
}
71+
$this->db->commit();
72+
} catch (\Throwable $e) {
73+
$this->db->rollBack();
74+
throw $e;
6875
}
6976
}
7077

‎lib/Migration/Version030800Date20250812122830.php

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,6 @@ public function __construct(
2121
) {
2222
}
2323

24-
/**
25-
* @param IOutput $output
26-
* @param Closure(): ISchemaWrapper $schemaClosure
27-
* @param array $options
28-
*/
29-
public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
30-
}
31-
3224
/**
3325
* @param IOutput $output
3426
* @param Closure(): ISchemaWrapper $schemaClosure
@@ -55,10 +47,10 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
5547
'notnull' => true
5648
]);
5749
$table->addColumn('pool', Types::BOOLEAN, [
58-
'notnull' => false,
59-
'default' => false
50+
'notnull' => true,
6051
]);
6152
$table->setPrimaryKey(['id']);
53+
$table->addIndex(['type'], 'oai_rule_type');
6254
}
6355
if (!$schema->hasTable('openai_quota_user')) {
6456
$table = $schema->createTable('openai_quota_user');
@@ -93,12 +85,4 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
9385

9486
return $schema;
9587
}
96-
97-
/**
98-
* @param IOutput $output
99-
* @param Closure $schemaClosure
100-
* @param array $options
101-
*/
102-
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
103-
}
10488
}

‎lib/Service/QuotaRuleService.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@
1212
use OCA\OpenAi\Db\QuotaRuleMapper;
1313
use OCA\OpenAi\Db\QuotaUserMapper;
1414
use OCP\AppFramework\Db\DoesNotExistException;
15+
use OCP\AppFramework\Db\MultipleObjectsReturnedException;
1516
use OCP\ICacheFactory;
1617
use OCP\IGroupManager;
1718
use OCP\IUserManager;
19+
use Psr\Log\LoggerInterface;
1820

1921
class QuotaRuleService {
2022
public function __construct(
@@ -24,14 +26,15 @@ public function __construct(
2426
private IGroupManager $groupManager,
2527
private ICacheFactory $cacheFactory,
2628
private IUserManager $userManager,
29+
private LoggerInterface $logger,
2730
) {
2831
}
2932
/**
3033
* Returns the quota rule for the given user
3134
*
3235
* @param int $quotaType
3336
* @param string $userId
34-
* @return ?array
37+
* @return array
3538
*/
3639
public function getRule(int $quotaType, string $userId) {
3740
$cache = $this->cacheFactory->createDistributed(Application::APP_ID);
@@ -42,7 +45,7 @@ public function getRule(int $quotaType, string $userId) {
4245
$groups = $this->groupManager->getUserGroupIds($user);
4346
try {
4447
$rule = $this->quotaRuleMapper->getRule($quotaType, $userId, $groups)->jsonSerialize();
45-
} catch (DoesNotExistException) {
48+
} catch (DoesNotExistException|MultipleObjectsReturnedException) {
4649
$rule = [
4750
'amount' => $this->openAiSettingsService->getQuotas()[$quotaType],
4851
'pool' => false,
@@ -164,13 +167,15 @@ private function validateRuleBasics(array $rule): void {
164167
private function validateEntities(array $entities) {
165168
foreach ($entities as $e) {
166169
if (!is_array($e)) {
170+
$this->logger->warning('Invalid entity', $e);
167171
throw new Exception('Invalid entity');
168172
}
169173
$validTypes = [
170174
'user',
171175
'group',
172176
];
173177
if (!isset($e['entity_type'], $e['entity_id']) || !in_array($e['entity_type'], $validTypes) || !is_string($e['entity_id'])) {
178+
$this->logger->warning('Invalid entity', $e);
174179
throw new Exception('Invalid entity');
175180
}
176181
}

‎src/components/AdminSettings.vue

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@
495495
input-id="openai-tts-voices-select"
496496
@click="onInput()" />
497497
<div>
498-
<h2>
498+
<h2 style="margin: 10px 0 -10px 0">
499499
{{ t('integration_openai', 'Usage limits') }}
500500
</h2>
501501
<div class="line">
@@ -508,7 +508,7 @@
508508
{{ t('integration_openai', 'Usage quotas per time period') }}
509509
</h2>
510510
<NcNoteCard type="info">
511-
{{ t('integration_openai', 'A per-user quota for each quota type can be set. If the user has not provided their own API key, this quota will be enforced.') }}
511+
{{ t('integration_openai', 'A per-user quota for each quota type can be set. If the user has not provided their own API key, and a rule is not specified for this user or any of their groups, this quota will be enforced.') }}
512512
{{ t('integration_openai', '"0" means unlimited usage for a particular quota type.') }}
513513
</NcNoteCard>
514514
<!--Loop through all quota types and list an input for them on this line-->
@@ -546,6 +546,7 @@
546546
</tr>
547547
</tbody>
548548
</table>
549+
<h2>{{ t('integration_openai', 'Quota Rules') }}</h2>
549550
<QuotaRules :quota-info="quotaInfo" />
550551
</div>
551552
<div>
@@ -983,4 +984,7 @@ export default {
983984
margin: 0 !important;
984985
}
985986
}
987+
.notecard {
988+
max-width: 900px;
989+
}
986990
</style>

0 commit comments

Comments
 (0)