Page MenuHomePhorge
Differential D2845

Greylist/Request simplify logic
ClosedPublic
Actions

Authored by mollekopf on Aug 27 2021, 11:50 AM.

Details

Reviewers
machniak
Group Reviewers
Restricted Project
Commits
rKa4534ed6c24f: Greylist/Request: simplify logic
Summary
  • There can only every be one entry for the sender/recipient/net pair (due to the unique constraint)
  • We already verify in the beginning that we have an entry within the last 7 days (and otherwise purge all entries), so it can't be older than a month.

Diff Detail

Repository
rK kolab
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 36505
Build 14488: arc lint + arc unit

Event Timeline

mollekopf requested review of this revision.Aug 27 2021, 11:50 AM
mollekopf created this revision.
Harbormaster completed remote builds in B36499: Diff 8182.Aug 27 2021, 11:50 AM
mollekopf added a reviewer: Restricted Project.Aug 27 2021, 11:51 AM
machniak accepted this revision as: machniak.Aug 27 2021, 11:56 AM
This revision is now accepted and ready to land.Aug 27 2021, 11:56 AM
mollekopf updated this revision to Diff 8188.Aug 27 2021, 11:56 AM

Reuse the existing function to query for the connection

Harbormaster completed remote builds in B36505: Diff 8188.Aug 27 2021, 11:56 AM
Closed by commit rKa4534ed6c24f: Greylist/Request: simplify logic. · Explain WhyAug 27 2021, 12:00 PM
This revision was automatically updated to reflect the committed changes.
machniak added inline comments.Aug 27 2021, 12:02 PM
src/app/Policy/Greylist/Request.php
188

We could skip the query if we know there's no entries, i.e. see line 87.

Revision Contents
Changeset List

PathSize
src/
app/
Policy/
Greylist/
24 lines

Diff 8188

View Options

src/app/Policy/Greylist/Request.php

Show First 20 Lines • Show All 1,963 Lines • ▼ Show 20 Lines protected function relations_state_get($device_key, $folderid, $synctime)
0, 0,
1, 1,
$device_key, $device_key,
$folderid, $folderid,
$synctime $synctime
); );
if ($row = $db->fetch_assoc()) { if ($row = $db->fetch_assoc()) {
// MariaDB's datetime output includes microseconds, we need to remove them, // Don't use $row['synctime'] for the internal cache.
// it must be "Y-m-d H:i:s" format // The synctime of the found row is usually earlier than the requested synctime.
$synctime = preg_replace('/\.[0-9]+/', '', $row['synctime']); // Note: We use internal cache because there's a call to both hasChanges() and
// getChangedEntries() in Sync. It's needed until we add some caching on a higher level.
$this->relations[$folderid][$synctime] = json_decode($row['data'], true); $this->relations[$folderid][$synctime] = json_decode($row['data'], true);
}
mollekopfUnsubmitted
Done
Inline Actions

This copies the latest database entry into the current $synctime cache, so that seems to indeed make more sense than reusing the synctime of the previous database entry.

mollekopf: This copies the latest database entry into the current $synctime cache, so that seems to indeed…
// Cleanup: remove all records except the current one // Cleanup: remove all records except the current one
$db->query( $db->query(
"DELETE FROM `syncroton_relations_state`" "DELETE FROM `syncroton_relations_state`"
. " WHERE `device_id` = ? AND `folder_id` = ? AND `synctime` <> ?", . " WHERE `device_id` = ? AND `folder_id` = ? AND `synctime` <> ?",
$device_key, $device_key,
$folderid, $folderid,
$synctime $row['synctime'],
mollekopfUnsubmitted
Not Done
Inline Actions

But this is supposed to remove all cache entries except the current one which identified by $synctime, so if $synctime and $row['synctime'] are different we would remove the current cache entry and instead leave the previous one, no? It may not matter because we just always get the latest.

mollekopf: But this is supposed to remove all cache entries except the current one which identified by…
mollekopfUnsubmitted
Not Done
Inline Actions

nevermind, "current" is not the new entry, but the one that latest that exists.

mollekopf: nevermind, "current" is not the new entry, but the one that latest that exists.
); );
} }
}
return $this->relations[$folderid][$synctime] ?? null; return $this->relations[$folderid][$synctime] ?? null;
} }
/** /**
* Return last storage error * Return last storage error
*/ */
public static function last_error() public static function last_error()
Show All 34 Lines