Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
makepot: "true"
services:
postgres:
image: postgres:13
image: postgis/postgis:13-3.4
env:
POSTGRES_USER: odoo
POSTGRES_PASSWORD: odoo
Expand Down
10 changes: 5 additions & 5 deletions base_geoengine/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ Geospatial support for Odoo
:target: http://www.gnu.org/licenses/agpl-3.0-standalone.html
:alt: License: AGPL-3
.. |badge3| image:: https://img.shields.io/badge/github-OCA%2Fgeospatial-lightgray.png?logo=github
:target: https://github.com/OCA/geospatial/tree/18.0/base_geoengine
:target: https://github.com/OCA/geospatial/tree/19.0/base_geoengine
:alt: OCA/geospatial
.. |badge4| image:: https://img.shields.io/badge/weblate-Translate%20me-F47D42.png
:target: https://translation.odoo-community.org/projects/geospatial-18-0/geospatial-18-0-base_geoengine
:target: https://translation.odoo-community.org/projects/geospatial-19-0/geospatial-19-0-base_geoengine
:alt: Translate me on Weblate
.. |badge5| image:: https://img.shields.io/badge/runboat-Try%20me-875A7B.png
:target: https://runboat.odoo-community.org/builds?repo=OCA/geospatial&target_branch=18.0
:target: https://runboat.odoo-community.org/builds?repo=OCA/geospatial&target_branch=19.0
:alt: Try me on Runboat

|badge1| |badge2| |badge3| |badge4| |badge5|
Expand Down Expand Up @@ -306,7 +306,7 @@ Bug Tracker
Bugs are tracked on `GitHub Issues <https://github.com/OCA/geospatial/issues>`_.
In case of trouble, please check there if your issue has already been reported.
If you spotted it first, help us to smash it by providing a detailed and welcomed
`feedback <https://github.com/OCA/geospatial/issues/new?body=module:%20base_geoengine%0Aversion:%2018.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.
`feedback <https://github.com/OCA/geospatial/issues/new?body=module:%20base_geoengine%0Aversion:%2019.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.

Do not contact contributors directly about support or help with technical issues.

Expand Down Expand Up @@ -363,6 +363,6 @@ OCA, or the Odoo Community Association, is a nonprofit organization whose
mission is to support the collaborative development of Odoo features and
promote its widespread use.

This module is part of the `OCA/geospatial <https://github.com/OCA/geospatial/tree/18.0/base_geoengine>`_ project on GitHub.
This module is part of the `OCA/geospatial <https://github.com/OCA/geospatial/tree/19.0/base_geoengine>`_ project on GitHub.

You are welcome to contribute. To learn how please visit https://odoo-community.org/page/Contribute.
1 change: 1 addition & 0 deletions base_geoengine/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@
from . import geo_convertion_helper
from . import geo_operators
from .geo_db import init_postgis
from . import domains
2 changes: 1 addition & 1 deletion base_geoengine/__manifest__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
{
"name": "Geospatial support for Odoo",
"version": "18.0.1.0.1",
"version": "19.0.1.0.1",
"category": "GeoBI",
"author": "Camptocamp,ACSONE SA/NV,Odoo Community Association (OCA)",
"license": "AGPL-3",
Expand Down
149 changes: 149 additions & 0 deletions base_geoengine/domains.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
import contextlib
import logging
import warnings

from odoo.fields import Domain
from odoo.models import BaseModel
from odoo.orm import domains
from odoo.orm.domains import (
CONDITION_OPERATORS,
NEGATIVE_CONDITION_OPERATORS,
SQL,
DomainCondition,
OptimizationLevel,
Query,
)
from odoo.orm.identifiers import NewId

_logger = logging.getLogger(__name__)

GEO_OPERATORS = frozenset(
[
"geo_greater",
"geo_lesser",
"geo_equal",
"geo_touch",
"geo_within",
"geo_contains",
"geo_intersect",
]
)


def checked(self) -> DomainCondition:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider wrapping the original methods and only handling GEO_OPERATORS as a special case, falling back to the originals for all other logic.

You don’t need to re-implement the entire `checked`/`_to_sql`/`_optimize_step` logic—just wrap the original methods and handle your GEO_OPERATORS case first, falling back immediately to the saved originals. This cuts out all the copied boilerplate and keeps behavior unchanged.

```python
# save originals
_orig_checked = DomainCondition.checked
_orig_to_sql  = DomainCondition._to_sql
_orig_optimize = DomainCondition._optimize_step

GEO_OPERATORS = frozenset({
    "geo_greater", "geo_lesser", "geo_equal", "geo_touch",
    "geo_within", "geo_contains", "geo_intersect",
})

def checked(self) -> DomainCondition:
    op = self.operator.lower()
    if op in GEO_OPERATORS:
        # (optional) warn if upper-case
        if op != self.operator:
            warnings.warn(
                f"geo operator should be lower-case: {(self.field_expr, self.operator)!r}",
                DeprecationWarning, stacklevel=2
            )
            return DomainCondition(self.field_expr, op, self.value).checked()
        return self
    return _orig_checked(self)

def _to_sql(self, model, alias, query) -> SQL:
    if self.operator in GEO_OPERATORS:
        assert self._opt_level >= OptimizationLevel.FULL, "Must optimize GEO first"
        fld = self._field(model)
        model._check_field_access(fld, "read")
        return fld.condition_to_sql(
            self.field_expr, self.operator, self.value, model, alias, query
        )
    return _orig_to_sql(self, model, alias, query)

def _optimize_step(self, model, level) -> Domain:
    if self.operator in GEO_OPERATORS:
        # bump level to FULL
        optimized = DomainCondition(self.field_expr, self.operator, self.value)
        object.__setattr__(optimized, "_opt_level", OptimizationLevel.FULL)
        return optimized
    return _orig_optimize(self, model, level)

# patch
DomainCondition.checked       = checked
DomainCondition._to_sql       = _to_sql
DomainCondition._optimize_step = _optimize_step

# expand the global set so `checked` sees them as valid
domains.CONDITION_OPERATORS |= GEO_OPERATORS

Benefits:

  • no full copy of Odoo’s original logic
  • clear “if GEO” / “else original” structure
  • all existing behavior (and warnings) are preserved
  • greatly reduced maintenance surface

"""Validate `self` and return it if correct, otherwise raise an exception."""
if not isinstance(self.field_expr, str) or not self.field_expr:
self._raise("Empty field name", error=TypeError)
operator = self.operator.lower()
if operator != self.operator:
warnings.warn(
(
f"Deprecated since 19.0, the domain condition "
f"{(self.field_expr, self.operator, self.value)!r} "
f"should have a lower-case operator"
),
DeprecationWarning,
# <MOD>
stacklevel=2,
# </MOD>
)
return DomainCondition(self.field_expr, operator, self.value).checked()
if operator not in CONDITION_OPERATORS:
# <MOD>
if operator not in GEO_OPERATORS:
# </MOD>
self._raise("Invalid operator")

# check already the consistency for domain manipulation
# these are common mistakes and optimizations,
# do them here to avoid recreating the domain
# - NewId is not a value
# - records are not accepted, use values
# - Query and Domain values should be using a relational operator
# <MOD>
# from .models import BaseModel # noqa: PLC0415
# </MOD>

value = self.value
if value is None:
value = False
elif isinstance(value, NewId):
_logger.warning(
"Domains don't support NewId, use .ids instead, for %r",
(self.field_expr, self.operator, self.value),
)
operator = "not in" if operator in NEGATIVE_CONDITION_OPERATORS else "in"
value = []
elif isinstance(value, BaseModel):
_logger.warning(
"The domain condition %r should not have a value which is a model",
(self.field_expr, self.operator, self.value),
)
value = value.ids
elif isinstance(value, (Domain, Query, SQL)) and operator not in (
"any",
"not any",
"any!",
"not any!",
"in",
"not in",
):
# accept SQL object in the right part for simple operators
# use case: compare 2 fields
_logger.warning(
"The domain condition %r should use the 'any' or 'not any' operator.",
(self.field_expr, self.operator, self.value),
)
if value is not self.value:
return DomainCondition(self.field_expr, operator, value)
return self


def _to_sql(self, model: BaseModel, alias: str, query: Query) -> SQL:
"""Enhanced _to_sql that handles geospatial operators."""
field_expr, operator, value = self.field_expr, self.operator, self.value

# Only handle geospatial operators here, delegate everything else to original method
if operator in GEO_OPERATORS:
# Ensure geospatial conditions are fully optimized
assert self._opt_level >= OptimizationLevel.FULL, (
"Must fully optimize before generating the query "
f"{(field_expr, operator, value)}"
)

field = self._field(model)
model._check_field_access(field, "read")
return field.condition_to_sql(field_expr, operator, value, model, alias, query)

# For all other operators, use the original method
return original__to_sql(self, model, alias, query)


def _optimize_step(self, model: BaseModel, level: OptimizationLevel) -> Domain:
"""Optimization step for geospatial operators."""
# For geospatial operators, we need to handle them specially during optimization
# If this is a geospatial operator, mark it as optimized at FULL level
if self.operator in GEO_OPERATORS:
# Perform basic validation and normalization
with contextlib.suppress(Exception):
field = self._field(model)
# Basic geospatial operator validation
if hasattr(field, "geo_type"): # It's a geospatial field
Comment on lines +122 to +131
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Custom _optimize_step for geospatial operators may not cover all edge cases.

Please enhance validation and error handling to ensure all field types and invalid configurations are properly managed.

# Create optimized version with FULL level
optimized = DomainCondition(self.field_expr, self.operator, self.value)
object.__setattr__(optimized, "_opt_level", OptimizationLevel.FULL)
return optimized

# Fall back to original optimization for non-geo operators
return original__optimize_step(self, model, level)


# Store original methods before monkey patching
original__optimize_step = DomainCondition._optimize_step
original__to_sql = DomainCondition._to_sql

DomainCondition.checked = checked
DomainCondition._to_sql = _to_sql
DomainCondition._optimize_step = _optimize_step

domains.CONDITION_OPERATORS = domains.CONDITION_OPERATORS.union(GEO_OPERATORS)
72 changes: 45 additions & 27 deletions base_geoengine/expressions.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
# Copyright 2023 ACSONE SA/NV
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

import logging
import random
import string

from odoo import fields
from odoo.fields import Domain
from odoo.models import BaseModel
from odoo.osv import expression
from odoo.osv.expression import TERM_OPERATORS
from odoo.tools import SQL, Query

from .fields import GeoField
from .geo_operators import GeoOperator

original___condition_to_sql = BaseModel._condition_to_sql
logger = logging.getLogger(__name__)

original___condition_to_sql = fields.Field._condition_to_sql

GEO_OPERATORS = {
"geo_greater": ">",
Expand All @@ -23,6 +26,7 @@
"geo_contains": "ST_Contains",
"geo_intersect": "ST_Intersects",
}

GEO_SQL_OPERATORS = {
"geo_greater": SQL(">"),
"geo_lesser": SQL("<"),
Expand All @@ -32,23 +36,23 @@
"geo_contains": SQL("ST_Contains"),
"geo_intersect": SQL("ST_Intersects"),
}
term_operators_list = list(TERM_OPERATORS)
for op in GEO_OPERATORS:
term_operators_list.append(op)

expression.TERM_OPERATORS = tuple(term_operators_list)
expression.SQL_OPERATORS.update(GEO_SQL_OPERATORS)


def _condition_to_sql(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider refactoring _condition_to_sql by delegating geo-operator logic to helpers and using SQL templates instead of manual string manipulation.

Here are a few targeted refactorings to flatten _condition_to_sql, remove raw mogrify/replace, and keep all functionality:

  1. Delegate all the geo‐dict logic to a private helper, returning a single SQL
  2. Replace manual mogrify + string‐replace with a proper EXISTS(%s) SQL template
  3. Only patch the geo‐operators path, otherwise call super()
# 1) Top‐level monkey‐patch keeps only geo‐operator check
def _condition_to_sql(self, field_expr, operator, value, model, alias, query):
    if operator.startswith("geo_"):
        return self._geo_to_sql(field_expr, operator, value, model, alias, query)
    return super(fields.Field, self)._condition_to_sql(
        field_expr=field_expr, operator=operator, value=value,
        model=model, alias=alias, query=query,
    )

# 2) Extracted helper for all geo operators
def _geo_to_sql(self, field_expr, operator, value, model, alias, query):
    field = model._fields[field_expr]
    assert isinstance(field, GeoField), "only on geo fields"
    geo = GeoOperator(field)
    params = []
    if isinstance(value, dict):
        return self._geo_dict_to_sql(field_expr, operator, value, model, alias)
    # simple geo_ operator
    sql_code = getattr(geo, f"get_{operator}_sql")(model._table, field_expr, value, params)
    return SQL(sql_code, *params)

# 3) Helper for the dict‐of‐related branch
def _geo_dict_to_sql(self, field_expr, operator, ref_search, model, alias):
    exists_clauses = []
    for rel_name, domain in ref_search.items():
        rel_model = model.env[rel_name]
        rel_alias = f"{rel_model._table}_{uuid4().hex[:5]}"
        rel_query = where_calc(rel_model, domain, active_test=True, alias=rel_alias)
        model._check_field_access(model._fields[field_expr], "read")
        # add the spatial predicate
        if operator in ("geo_greater", "geo_lesser"):
            rel_query.add_where(
                SQL("ST_Area(%s.%s) %s ST_Area(%s.%s)",
                    alias, field_expr,
                    GEO_SQL_OPERATORS[operator],
                    rel_alias, rel_name.split(".")[-1],
                )
            )
        else:
            rel_query.add_where(
                SQL("%s(%s.%s, %s.%s)",
                    GEO_SQL_OPERATORS[operator],
                    alias, field_expr,
                    rel_alias, rel_name.split(".")[-1],
                )
            )
        sub = rel_query.subselect("1")
        # use the SQL object directly instead of .mogrify + replace
        exists_clauses.append(SQL("EXISTS(%s)", sub))
    return SQL(" AND ").join(exists_clauses)

• Now the main patch is only ~20 lines
• We never manually manipulate the raw query string, rely on SQL() and its params
• All geo‐dict logic is isolated in _geo_dict_to_sql
• The default path simply calls the original super—no duplication of Odoo’s core logic.

self, alias: str, fname: str, operator: str, value, query: Query
self,
field_expr: str,
operator: str,
value,
model: BaseModel,
alias: str,
query: Query,
) -> SQL:
"""
This method has been monkey patched in order to be able to include
geo_operators into the Odoo search method.
"""
if operator in GEO_OPERATORS.keys():
current_field = self._fields.get(fname)
current_field = model._fields.get(field_expr)
current_operator = GeoOperator(current_field)
if current_field and isinstance(current_field, GeoField):
params = []
Expand All @@ -59,9 +63,9 @@ def _condition_to_sql(
sub_queries = []
for key in ref_search:
i = key.rfind(".")
rel_model = key[0:i]
rel_model_name = key[0:i]
rel_col = key[i + 1 :]
rel_model = self.env[rel_model]
rel_model = model.env[rel_model_name]
# we compute the attributes search on spatial rel
if ref_search[key]:
rel_alias = (
Expand All @@ -75,42 +79,52 @@ def _condition_to_sql(
active_test=True,
alias=rel_alias,
)
self._apply_ir_rules(rel_query, "read")
model._check_field_access(current_field, "read")
if operator == "geo_equal":
rel_query.add_where(
f'"{alias}"."{fname}" {GEO_OPERATORS[operator]} '
f'"{alias}"."{field_expr}" {GEO_OPERATORS[operator]} '
f"{rel_alias}.{rel_col}"
)
elif operator in ("geo_greater", "geo_lesser"):
rel_query.add_where(
f"ST_Area({alias}.{fname}) {GEO_OPERATORS[operator]} "
f"ST_Area({alias}.{field_expr}) "
f"{GEO_OPERATORS[operator]} "
f"ST_Area({rel_alias}.{rel_col})"
)
else:
rel_query.add_where(
f'{GEO_OPERATORS[operator]}("{alias}"."{fname}", '
f'{GEO_OPERATORS[operator]}("{alias}"."{field_expr}", '
f"{rel_alias}.{rel_col})"
)

subquery, subparams = rel_query.subselect("1")
subquery_sql = rel_query.subselect("1")
sub_query_mogrified = (
self.env.cr.mogrify(subquery, subparams)
model.env.cr.mogrify(subquery_sql.code, subquery_sql.params)
.decode("utf-8")
.replace(f"'{rel_model._table}'", f'"{rel_model._table}"')
.replace("%", "%%")
)
sub_queries.append(f"EXISTS({sub_query_mogrified})")
query = " AND ".join(sub_queries)
query_str = " AND ".join(sub_queries)
else:
query = get_geo_func(
current_operator, operator, fname, value, params, self._table
query_str = get_geo_func(
current_operator, operator, field_expr, value, params, model._table
)
return SQL(query, *params)
return SQL(query_str, *params)
return original___condition_to_sql(
self, alias=alias, fname=fname, operator=operator, value=value, query=query
self,
field_expr=field_expr,
operator=operator,
value=value,
model=model,
alias=alias,
query=query,
)


fields.Field._condition_to_sql = _condition_to_sql


def get_geo_func(current_operator, operator, left, value, params, table):
"""
This method will call the SQL query corresponding to the requested geo operator
Expand Down Expand Up @@ -149,8 +163,12 @@ def where_calc(model, domain, active_test=True, alias=None):

query = Query(model.env, alias, model._table)
if domain:
return expression.expression(domain, model, alias=alias, query=query).query
return query
# In Odoo 19, create Domain object and use its _to_sql method
domain_obj = Domain(domain)
optimized_domain = domain_obj.optimize_full(model)
sql_condition = optimized_domain._to_sql(model, alias, query)
query.add_where(sql_condition)

return query

BaseModel._condition_to_sql = _condition_to_sql
return query
Loading