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
65 changes: 56 additions & 9 deletions backend/backend/application/session/connection_session.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import logging
from typing import Any

from visitran.utils import import_file

from backend.application.utils import get_filter

logger = logging.getLogger(__name__)
from backend.core.models.connection_models import ConnectionDetails
from backend.core.models.environment_models import EnvironmentModels
from backend.core.models.project_details import ProjectDetails
Expand All @@ -14,6 +17,32 @@
from backend.utils.pagination import CustomPaginator


def _get_host_display(con_model):
"""Extract a human-readable host string from connection details.

Only reads non-sensitive plaintext fields (host, port, account, etc.)
so no Fernet decryption is needed.
"""
try:
details = con_model.connection_details or {}
ds = con_model.datasource_name
if ds in ("postgres", "mysql", "trino"):
host = details.get("host", "")
port = details.get("port", "")
return f"{host}:{port}" if host and port else host or None
if ds == "snowflake":
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Must Fix: _get_host_display() decrypts passwords/tokens unnecessarily

con_model.decrypted_connection_details runs Fernet decryption on every sensitive field (password, api_key, token, connection_url, etc.) — but this function only reads non-sensitive fields like host, port, account, project_id.

These fields are stored as plaintext in the DB (only fields in SENSITIVE_FIELDS are encrypted). So you can read connection_details directly and skip decryption entirely:

def _get_host_display(con_model):
    try:
        details = con_model.connection_details  # raw dict, no decryption needed
        ds = con_model.datasource_name
        if ds in ("postgres", "mysql", "trino"):
            host = details.get("host", "")
            port = details.get("port", "")
            return f"{host}:{port}" if host and port else host or None
        if ds == "snowflake":
            return details.get("account") or None
        if ds == "bigquery":
            return details.get("project_id") or None
        if ds == "databricks":
            return details.get("host") or None
        if ds == "duckdb":
            return details.get("file_path") or None
    except Exception:
        pass
    return None

This eliminates Fernet CPU overhead on every row in the list API (20 connections × decryption = noticeable on page load).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in f3e25aa — now reads connection_details directly (the raw JSON) instead of decrypted_connection_details. Host, port, account, project_id are all plaintext — no Fernet decryption needed.

return details.get("account") or None
if ds == "bigquery":
return details.get("project_id") or None
if ds == "databricks":
return details.get("host") or None
if ds == "duckdb":
return details.get("file_path") or None
except Exception:
logger.warning("Failed to derive host display for connection %s", con_model.connection_id, exc_info=True)
return None


class ConnectionSession:

@staticmethod
Expand Down Expand Up @@ -43,35 +72,53 @@ def create_connection(connection_details: dict[str, Any]) -> ConnectionDetails:

@staticmethod
def get_all_connections(page: int, limit: int, filter_condition: dict[str, Any]) -> Any:
from django.db.models import Count, Q, Exists, OuterRef

filter_condition.update(get_filter())
if "is_deleted" not in filter_condition:
filter_condition["is_deleted"] = False
con_models = ConnectionDetails.objects.filter(**filter_condition).order_by("-modified_at")

custom_paginator = CustomPaginator(queryset=con_models, limit=limit, page=page)
# Annotate counts + sample flag in a single query (no N+1)
con_qs = (
ConnectionDetails.objects.filter(**filter_condition)
.annotate(
env_count=Count(
"environment_model",
filter=Q(environment_model__is_deleted=False),
distinct=True,
),
project_count=Count("project", distinct=True),
is_sample=Exists(
ProjectDetails.objects.filter(
connection_model_id=OuterRef("connection_id"),
is_sample=True,
)
),
)
.order_by("-modified_at")
)

custom_paginator = CustomPaginator(queryset=con_qs, limit=limit, page=page)
con_models = custom_paginator.paginate()

connection_list = []
for con_model in con_models.get("page_items"):
project_con = ProjectDetails.objects.filter(connection_model_id=con_model.connection_id).first()
if project_con:
is_sample_project = project_con.is_sample
else:
is_sample_project = False
connection_list.append(
{
"id": con_model.connection_id,
"name": con_model.connection_name,
"description": con_model.connection_description,
"datasource_name": con_model.datasource_name,
"host": _get_host_display(con_model),
"created_by": con_model.created_by,
"last_modified_by": con_model.last_modified_by,
"db_icon": import_file(f"visitran.adapters.{con_model.datasource_name}").ICON,
"is_connection_exist": con_model.is_connection_exist,
"is_connection_valid": con_model.is_connection_valid,
"connection_flag": con_model.connection_flag,
"is_sample_project": is_sample_project,
# "connection_details": con_model.connection_details, # skipping connection_details
"is_sample_project": con_model.is_sample,
"env_count": con_model.env_count,
"project_count": con_model.project_count,
}
)

Expand Down
34 changes: 27 additions & 7 deletions backend/backend/application/session/env_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from visitran.utils import import_file

from backend.application.session.connection_session import ConnectionSession
from backend.application.session.connection_session import ConnectionSession, _get_host_display
from backend.application.utils import get_filter
from backend.core.models.environment_models import EnvironmentModels
from backend.core.models.project_details import ProjectDetails
Expand Down Expand Up @@ -59,6 +59,7 @@ def create_environment(self, environment_details: dict[str, Any]) -> Environment
env_connection_data=env_connection_data,
env_custom_data=environment_details.get("custom_data", {}),
connection_model=connection_model,
is_tested=True,
)
env_model.save()
return env_model
Expand All @@ -82,26 +83,41 @@ def get_all_environment_models(filter_condition: dict[str, Any]) -> Any:
return env_models

def get_all_environments(self, page: int, limit: int, filter_condition: dict[str, Any]) -> Any:
env_models = self.get_all_environment_models(filter_condition=filter_condition).order_by("-modified_at")
from django.db.models import Count

env_qs = (
self.get_all_environment_models(filter_condition=filter_condition)
.select_related("connection_model")
.annotate(
job_count=Count("usertaskdetails", distinct=True),
project_count=Count("project", distinct=True),
)
.order_by("-modified_at")
)

custom_paginator = CustomPaginator(queryset=env_models, limit=limit, page=page)
custom_paginator = CustomPaginator(queryset=env_qs, limit=limit, page=page)
env_models = custom_paginator.paginate()

env_data = []
for env_model in env_models.get("page_items"):
conn = env_model.connection_model
env_data.append(
{
"id": env_model.environment_id,
"name": env_model.environment_name,
"description": env_model.environment_description,
"deployment_type": env_model.deployment_type,
"connection": {
"id": env_model.connection_model.connection_id,
"name": env_model.connection_model.connection_name,
"datasource_name": env_model.connection_model.datasource_name,
"db_icon": import_file(f"visitran.adapters.{env_model.connection_model.datasource_name}").ICON,
"id": conn.connection_id,
"name": conn.connection_name,
"datasource_name": conn.datasource_name,
"db_icon": import_file(f"visitran.adapters.{conn.datasource_name}").ICON,
"host": _get_host_display(conn),
"connection_flag": conn.connection_flag,
},
"is_tested": env_model.is_tested,
"job_count": env_model.job_count,
"project_count": env_model.project_count,
}
)
env_models["page_items"] = env_data
Expand All @@ -118,6 +134,9 @@ def get_environment(self, environment_id: str) -> dict[str, Any]:
"id": env_model.connection_model.connection_id,
"name": env_model.connection_model.connection_name,
"datasource_name": env_model.connection_model.datasource_name,
"db_icon": import_file(f"visitran.adapters.{env_model.connection_model.datasource_name}").ICON,
"host": _get_host_display(env_model.connection_model),
"connection_flag": env_model.connection_model.connection_flag,
},
"connection_details": env_model.masked_connection_data,
"custom_data": env_model.env_custom_data,
Expand All @@ -136,6 +155,7 @@ def update_environment(self, environment_id: str, environment_details: dict[str,
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 is_tested set to True on every save regardless of test result

env_model.is_tested = True is appended unconditionally to the update path. When a user edits only the environment name or description (no credential change), the frontend's canSave allows saving without running a test — but the backend now stamps is_tested = True on the record. Any environment that previously showed "Untested" will be silently flipped to "Tested" after a name-only edit, corrupting the status tag in EnvList.

The fix is to only set is_tested = True when the credentials have actually been tested, or to preserve the existing value for metadata-only updates.

# Only promote is_tested when connection_details were explicitly included in the payload
if "connection_details" in environment_details:
    env_model.is_tested = True
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/backend/application/session/env_session.py
Line: 155

Comment:
**`is_tested` set to `True` on every save regardless of test result**

`env_model.is_tested = True` is appended unconditionally to the update path. When a user edits only the environment name or description (no credential change), the frontend's `canSave` allows saving without running a test — but the backend now stamps `is_tested = True` on the record. Any environment that previously showed "Untested" will be silently flipped to "Tested" after a name-only edit, corrupting the status tag in `EnvList`.

The fix is to only set `is_tested = True` when the credentials have actually been tested, or to preserve the existing value for metadata-only updates.

```python
# Only promote is_tested when connection_details were explicitly included in the payload
if "connection_details" in environment_details:
    env_model.is_tested = True
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

env_model.env_connection_data = env_connection_data
env_model.env_custom_data = environment_details.get("custom_data", {})
env_model.is_tested = True
env_model.save()
return env_model
except KeyError as e:
Expand Down
Loading
Loading