fix(db): Avoid dirty read for collected addresses
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
This commit is contained in:
Родитель
6c421bd6da
Коммит
66fdc0bbe0
|
@ -60,11 +60,7 @@ class CollectedAddressMapper extends QBMapper {
|
|||
return $this->findEntities($dbQuery);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param null|string $email
|
||||
* @return bool
|
||||
*/
|
||||
public function exists(string $userId, ?string $email) {
|
||||
public function insertIfNew(string $userId, string $email, ?string $label): bool {
|
||||
$qb = $this->db->getQueryBuilder();
|
||||
$dbQuery = $qb
|
||||
->select('*')
|
||||
|
@ -72,7 +68,18 @@ class CollectedAddressMapper extends QBMapper {
|
|||
->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId)))
|
||||
->andWhere($qb->expr()->iLike('email', $qb->createNamedParameter($email)));
|
||||
|
||||
return count($this->findEntities($dbQuery)) > 0;
|
||||
if (!empty($this->findEntities($dbQuery))) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$entity = new CollectedAddress();
|
||||
$entity->setUserId($userId);
|
||||
if ($label !== $email) {
|
||||
$entity->setDisplayName($label);
|
||||
}
|
||||
$entity->setEmail($email);
|
||||
$this->insert($entity);
|
||||
return true;
|
||||
}
|
||||
|
||||
public function getTotal(): int {
|
||||
|
|
|
@ -28,9 +28,12 @@ use OCA\Mail\Address;
|
|||
use OCA\Mail\AddressList;
|
||||
use OCA\Mail\Db\CollectedAddress;
|
||||
use OCA\Mail\Db\CollectedAddressMapper;
|
||||
use OCP\AppFramework\Db\TTransactional;
|
||||
use Psr\Log\LoggerInterface;
|
||||
|
||||
class AddressCollector {
|
||||
use TTransactional;
|
||||
|
||||
/** @var CollectedAddressMapper */
|
||||
private $mapper;
|
||||
|
||||
|
@ -78,16 +81,8 @@ class AddressCollector {
|
|||
$this->logger->debug("<" . $address->getEmail() . "> is not a valid RFC822 mail address");
|
||||
return;
|
||||
}
|
||||
if ($address->getEmail() !== null && !$this->mapper->exists($userId, $address->getEmail())) {
|
||||
$this->logger->debug("saving new address <{$address->getEmail()}>");
|
||||
|
||||
$entity = new CollectedAddress();
|
||||
$entity->setUserId($userId);
|
||||
if ($address->getLabel() !== $address->getEmail()) {
|
||||
$entity->setDisplayName($address->getLabel());
|
||||
}
|
||||
$entity->setEmail($address->getEmail());
|
||||
$this->mapper->insert($entity);
|
||||
if ($address->getEmail() !== null && $this->mapper->insertIfNew($userId, $address->getEmail(), $address->getLabel())) {
|
||||
$this->logger->debug("saved new address <{$address->getEmail()}>");
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -125,18 +125,18 @@ class CollectedAddressMapperTest extends TestCase {
|
|||
}
|
||||
}
|
||||
|
||||
public function existsData() {
|
||||
public function insertIfNewData(): array {
|
||||
return [
|
||||
['user1@example.com', true],
|
||||
['user3@example.com', false],
|
||||
['user1@example.com', false],
|
||||
['user3@example.com', true],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider existsData
|
||||
* @dataProvider insertIfNewData
|
||||
*/
|
||||
public function testExists($email, $expected) {
|
||||
$actual = $this->mapper->exists($this->userId, $email);
|
||||
public function testExists($email, $expected): void {
|
||||
$actual = $this->mapper->insertIfNew($this->userId, $email, null);
|
||||
|
||||
$this->assertSame($expected, $actual);
|
||||
}
|
||||
|
|
|
@ -48,7 +48,7 @@ class AddressCollectorTest extends TestCase {
|
|||
);
|
||||
}
|
||||
|
||||
public function testAddAddresses() {
|
||||
public function testAddAddresses(): void {
|
||||
$addresses = [
|
||||
'"User" <user@example.com>',
|
||||
'Example <example@user.com>',
|
||||
|
@ -64,37 +64,29 @@ class AddressCollectorTest extends TestCase {
|
|||
$address2->setUserId($this->userId);
|
||||
|
||||
$this->mapper->expects($this->exactly(2))
|
||||
->method('exists')
|
||||
->method('insertIfNew')
|
||||
->withConsecutive(
|
||||
[$this->userId, 'user@example.com'],
|
||||
[$this->userId, 'example@user.com']
|
||||
)
|
||||
->willReturnOnConsecutiveCalls(
|
||||
false,
|
||||
false
|
||||
);
|
||||
$this->mapper->expects($this->exactly(2))
|
||||
->method('insert')
|
||||
->withConsecutive(
|
||||
[$address1],
|
||||
[$address2]
|
||||
true,
|
||||
true,
|
||||
);
|
||||
|
||||
$this->collector->addAddresses($this->userId, $addressList);
|
||||
}
|
||||
|
||||
public function testAddDuplicateAddresses() {
|
||||
public function testAddDuplicateAddresses(): void {
|
||||
$addresses = [
|
||||
'user@example.com',
|
||||
];
|
||||
$addressList = AddressList::parse($addresses);
|
||||
|
||||
$this->mapper->expects($this->once())
|
||||
->method('exists')
|
||||
->method('insertIfNew')
|
||||
->with($this->userId, $addresses[0])
|
||||
->will($this->returnValue(true));
|
||||
$this->mapper->expects($this->never())
|
||||
->method('insert');
|
||||
->willReturn(false);
|
||||
|
||||
$this->collector->addAddresses($this->userId, $addressList);
|
||||
}
|
||||
|
|
Загрузка…
Ссылка в новой задаче