Changeset View
Changeset View
Standalone View
Standalone View
swh/storage/db.py
Show First 20 Lines • Show All 572 Lines • ▼ Show 20 Lines | ) -> Optional[Dict[str, Any]]: | ||||
query_parts.append("ORDER BY ovs.date DESC LIMIT 1") | query_parts.append("ORDER BY ovs.date DESC LIMIT 1") | ||||
query = "\n".join(query_parts) | query = "\n".join(query_parts) | ||||
cur.execute(query, tuple(query_params)) | cur.execute(query, tuple(query_params)) | ||||
row = cur.fetchone() | row = cur.fetchone() | ||||
return self._make_origin_visit_status(row) | return self._make_origin_visit_status(row) | ||||
def origin_visit_get_all(self, origin_id, last_visit=None, limit=None, cur=None): | def origin_visit_get_all( | ||||
self, origin_id, last_visit=None, order="asc", limit=None, cur=None | |||||
): | |||||
"""Retrieve all visits for origin with id origin_id. | """Retrieve all visits for origin with id origin_id. | ||||
Args: | Args: | ||||
origin_id: The occurrence's origin | origin_id: The occurrence's origin | ||||
Yields: | Yields: | ||||
The visits for that origin | The visits for that origin | ||||
""" | """ | ||||
cur = self._cursor(cur) | cur = self._cursor(cur) | ||||
vlorentz: don't use assertions for sanitization.
Running Python in optimized mode (`-O`) doesn't run… | |||||
Done Inline Actionswell it's not a big deal because we're unlikely to ever do that, but I really don't like substituting a parameter in the query. I'd feel better with something like: if order == 'asc': query = "ORDER BY ov.visit ASC" elif order == 'desc': query = "ORDER BY ov.visit DESC" else: assert False even if it's redundant vlorentz: well it's not a big deal because we're unlikely to ever do that, but I really don't like… | |||||
Done Inline ActionsDamn... thanks. What should i replace it with then? if order.lower() not in ALLOWED_ORDER: raise ValueError(f"Order parameter must be one of {','.join(ALLOWED_ORDER)}") or something? ardumont: Damn... thanks.
What should i replace it with then?
```
if order.lower() not in ALLOWED_ORDER… | |||||
Not Done Inline Actionsno need for a constant, but yeah that's fine vlorentz: no need for a constant, but yeah that's fine | |||||
Done Inline ActionsI did as your first proposal in the end ;) ardumont: I did as your first proposal in the end ;) | |||||
assert order.lower() in ["asc", "desc"] | |||||
if last_visit: | query_parts = [ | ||||
extra_condition = "and ov.visit > %s" | "SELECT DISTINCT ON (ov.visit) %s " | ||||
args = (origin_id, last_visit, limit) | % ", ".join(self.origin_visit_select_cols), | ||||
else: | "FROM origin_visit ov", | ||||
extra_condition = "" | "INNER JOIN origin o ON o.id = ov.origin", | ||||
args = (origin_id, limit) | "INNER JOIN origin_visit_status ovs", | ||||
"ON ov.origin = ovs.origin AND ov.visit = ovs.visit", | |||||
] | |||||
query_parts.append("WHERE o.url = %s") | |||||
query_params: List[Any] = [origin_id] | |||||
Done Inline Actionssame as for cassandra, how does this deal with pagination when order is desc? douardda: same as for cassandra, how does this deal with pagination when order is desc? | |||||
Done Inline Actionsnow it does ;) ardumont: now it does ;) | |||||
query = """\ | if last_visit is not None: | ||||
SELECT DISTINCT ON (ov.visit) %s | op_comparison = ">" if order == "asc" else "<" | ||||
FROM origin_visit ov | query_parts.append(f"and ov.visit {op_comparison} %s") | ||||
INNER JOIN origin o ON o.id = ov.origin | query_params.append(last_visit) | ||||
INNER JOIN origin_visit_status ovs | |||||
ON ov.origin = ovs.origin AND ov.visit = ovs.visit | if order == "asc": | ||||
WHERE o.url=%%s %s | query_parts.append("ORDER BY ov.visit ASC, ovs.date DESC") | ||||
ORDER BY ov.visit ASC, ovs.date DESC | elif order == "desc": | ||||
LIMIT %%s""" % ( | query_parts.append("ORDER BY ov.visit DESC, ovs.date DESC") | ||||
", ".join(self.origin_visit_select_cols), | else: | ||||
extra_condition, | assert False | ||||
) | |||||
cur.execute(query, args) | if limit is not None: | ||||
query_parts.append("LIMIT %s") | |||||
query_params.append(limit) | |||||
query = "\n".join(query_parts) | |||||
cur.execute(query, tuple(query_params)) | |||||
yield from cur | yield from cur | ||||
def origin_visit_get(self, origin_id, visit_id, cur=None): | def origin_visit_get(self, origin_id, visit_id, cur=None): | ||||
"""Retrieve information on visit visit_id of origin origin_id. | """Retrieve information on visit visit_id of origin origin_id. | ||||
Args: | Args: | ||||
origin_id: the origin concerned | origin_id: the origin concerned | ||||
visit_id: The visit step for that origin | visit_id: The visit step for that origin | ||||
▲ Show 20 Lines • Show All 634 Lines • Show Last 20 Lines |
don't use assertions for sanitization.
Running Python in optimized mode (-O) doesn't run assertions, so callers could inject SQL via that parameter.