Skip to content

Commit

Permalink
trim logs and results, add cache
Browse files Browse the repository at this point in the history
  • Loading branch information
mutantsan committed Jul 29, 2024
1 parent 34fa166 commit adb6075
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 60 deletions.
68 changes: 61 additions & 7 deletions ckanext/bulk/assets/scripts/bulk-manager-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ ckan.module("bulk-manager-form", function () {
addFilter: "/bulk/htmx/create_filter_item",
addUpdate: "/bulk/htmx/create_update_item",
},
options: {},
options: {
resultMaxEntries: 50,
logsMaxEntries: 50,
},

initialize() {
$.proxyAll(this, /_/);
Expand Down Expand Up @@ -205,7 +208,6 @@ ckan.module("bulk-manager-form", function () {
}

this._toggleLoadSpinner(true);
// window.bulkUpdateOn = this._getUpdateOn();

if (!data.filters.length) {
this.infoBlock.find(".counter").html("There will be information about how many entities will be changed.");
Expand All @@ -217,7 +219,7 @@ ckan.module("bulk-manager-form", function () {
"bulk_get_entities_by_filters",
data,
(data) => {
if (!data.result || data.result.error || data.result.fields.length === 0) {
if (!data.result || data.result.error || data.result.entities.length === 0) {
if (data.result.error) {
iziToast.error({ message: data.result.error });
}
Expand All @@ -231,14 +233,14 @@ ckan.module("bulk-manager-form", function () {
this.bulkModalResult.iziModal(
"setContent",
"<pre class='language-javascript'>"
+ JSON.stringify(data.result.fields, null, 2)
+ JSON.stringify(this._limitResultEntries(data.result.entities), null, 2)
+ "</pre>"
);

Prism.highlightElement(this.bulkModalResult.find("pre")[0]);

this.bulkEntitiesToUpdate = data.result.fields;
this.infoBlock.find(".counter").html("Found " + data.result.fields.length + " entities");
this.bulkEntitiesToUpdate = data.result.entities;
this.infoBlock.find(".counter").html("Found " + data.result.entities.length + " entities");
this._toggleLoadSpinner(false);
},
(resp) => {
Expand All @@ -248,6 +250,58 @@ ckan.module("bulk-manager-form", function () {
);
},

/**
* Limit the number of entries in the result to avoid performance issues.
*
* Also remove some fields that are not needed for the user.
*
* @param {Array<Object>} entities
*
* @returns {Array<Object>}
*/
_limitResultEntries: function (entities) {
entities = entities.slice(0, this.options.resultMaxEntries);

entities.forEach(entity => {
delete entity.resources;
delete entity.organization;
delete entity.groups;

delete entity.relationships_as_subject,
delete entity.relationships_as_object;
});

return entities;
},

/**
* Limit the number of entries in the logs to avoid performance issues.
*
* Also remove some fields that are not needed for the user.
*
* @param {Array<Object>} logs
*
* @returns {Array<Object>}
*/
_limitLogsEntries: function (logs) {
logs = logs.slice(logs.length - this.options.logsMaxEntries, logs.length);

logs.forEach(log => {
if (!log.result) {
return;
}

delete log.result.resources;
delete log.result.organization;
delete log.result.groups;

delete log.result.relationships_as_subject,
delete log.result.relationships_as_object;
});

return logs;
},

_initFieldSelectors: function (selectItems, reinit = false) {
let prevValue = "";

Expand Down Expand Up @@ -335,7 +389,7 @@ ckan.module("bulk-manager-form", function () {
"setContent",
{
content: "<pre class='language-javascript'>"
+ JSON.stringify(this.bulkLogs, null, 2)
+ JSON.stringify(this._limitLogsEntries(this.bulkLogs), null, 2)
+ "</pre>",
}
);
Expand Down
89 changes: 63 additions & 26 deletions ckanext/bulk/entity_manager.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
from __future__ import annotations

import json
from abc import abstractmethod
from typing import Any, TypedDict

from attr import field

import ckan.plugins.toolkit as tk
from ckan import model
from ckan.lib.redis import connect_to_redis

from ckanext.bulk import const

Expand Down Expand Up @@ -67,10 +71,10 @@ def get_entity_by_id(cls, entity_id: str) -> dict[str, Any] | None:

@classmethod
def combine_filters(cls, filters: list[FilterItem]) -> list[CombinedFilter]:
combined_filters = []
combined_filters: list[CombinedFilter] = []

current_field = None
current_operator = None
current_field = ""
current_operator = ""
current_values = []

for filter_item in filters:
Expand All @@ -82,11 +86,11 @@ def combine_filters(cls, filters: list[FilterItem]) -> list[CombinedFilter]:
else:
if current_field and current_operator:
combined_filters.append(
{
"field": current_field,
"operator": current_operator,
"values": current_values,
}
CombinedFilter(
field=current_field,
operator=current_operator,
value=current_values,
)
)

current_field = filter_item["field"]
Expand All @@ -95,11 +99,11 @@ def combine_filters(cls, filters: list[FilterItem]) -> list[CombinedFilter]:

if current_field and current_operator:
combined_filters.append(
{
"field": current_field,
"operator": current_operator,
"values": current_values,
}
CombinedFilter(
field=current_field,
operator=current_operator,
value=current_values,
)
)

return combined_filters
Expand All @@ -111,7 +115,7 @@ def update_entity(
entity = cls.get_entity_by_id(entity_id)

if not entity:
raise tk.ObjectNotFound(f"Entity {entity_id} not found")
raise tk.ObjectNotFound(f"Entity <{entity_id}> not found")

return tk.get_action(cls.patch_action)(
{"ignore_auth": True},
Expand All @@ -131,6 +135,22 @@ def delete_entity(cls, entity_id: str) -> bool:

return True

@classmethod
def cache_fields_to_redis(cls, fields: list[FieldItem], ttl: int = 3600):
conn = connect_to_redis()
conn.set(f"ckanext-bulk:fields:{cls.entity_type}", json.dumps(fields), ex=ttl)

@classmethod
def get_fields_from_redis(cls) -> list[FieldItem]:
conn = connect_to_redis()

fields = conn.get(f"ckanext-bulk:fields:{cls.entity_type}")

if not fields:
return []

return json.loads(fields)


class DatasetEntityManager(EntityManager):
entity_type = "dataset"
Expand All @@ -140,6 +160,9 @@ class DatasetEntityManager(EntityManager):

@classmethod
def get_fields(cls) -> list[FieldItem]:
if fields := cls.get_fields_from_redis():
return fields

result = tk.get_action("package_search")(
{"ignore_auth": True},
{"rows": 1, "include_private": True, "q": f'type:"{cls.entity_type}"'},
Expand All @@ -148,7 +171,11 @@ def get_fields(cls) -> list[FieldItem]:
if not result["results"]:
return []

return [FieldItem(value=field, text=field) for field in result["results"][0]]
fields = [FieldItem(value=field, text=field) for field in result["results"][0]]

cls.cache_fields_to_redis(fields)

return fields

@classmethod
def search_entities_by_filters(
Expand Down Expand Up @@ -190,7 +217,7 @@ def search_entities_by_filters(
query = f" {global_operator} ".join(q_list)

start = 0
rows = 100
rows = 1000

results = []

Expand Down Expand Up @@ -225,6 +252,9 @@ class DatasetResourceEntityManager(EntityManager):

@classmethod
def get_fields(cls) -> list[FieldItem]:
if fields := cls.get_fields_from_redis():
return fields

resource: model.Resource | None = (
model.Session.query(model.Resource)
.join(model.Package)
Expand All @@ -235,7 +265,13 @@ def get_fields(cls) -> list[FieldItem]:
if not resource:
return []

return [FieldItem(value=field, text=field) for field in resource.get_columns()]
fields = [
FieldItem(value=field, text=field) for field in resource.get_columns()
]

cls.cache_fields_to_redis(fields)

return fields

@classmethod
def search_entities_by_filters(
Expand Down Expand Up @@ -275,20 +311,21 @@ class GroupEntityManager(EntityManager):

@classmethod
def get_fields(cls) -> list[FieldItem]:
item_list = tk.get_action(cls.show_action)(
{"ignore_auth": True}, {"all_fields": True}
if fields := cls.get_fields_from_redis():
return fields

item_list: list[dict[str, Any]] = tk.get_action(cls.show_action)(
{"ignore_auth": True}, {"all_fields": True, "rows": 1}
)

if not item_list:
return []

return [
{
"value": field,
"text": field,
}
for field in item_list[0]
]
fields = [FieldItem(value=field, text=field) for field in item_list[0]]

cls.cache_fields_to_redis(fields)

return fields

@classmethod
def search_entities_by_filters(
Expand Down
6 changes: 3 additions & 3 deletions ckanext/bulk/logic/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,16 @@ def bulk_get_entities_by_filters(context: Context, data_dict: dict[str, Any]):
)
except (ValueError, tk.ValidationError) as e:
return {
"fields": [],
"entities": [],
"error": str(e),
}
except DatabaseError as e:
return {
"fields": [],
"entities": [],
"error": f"Database error: {e.statement}",
}

return {"fields": result}
return {"entities": result}


@tk.side_effect_free
Expand Down
24 changes: 0 additions & 24 deletions ckanext/bulk/tests/test_entity_managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -488,17 +488,6 @@ def test_update_group_invalid_field(self, group_entity_manager, group_factory):

assert "new_field" not in result

def test_update_group_empty_field(self, group_entity_manager, group_factory):
group = group_factory()

result = group_entity_manager.update_entity(
group["id"], [{"field": "name", "value": ""}]
)

# TODO: this looks like a CKAN bug, we shouldn't be able to nullify
# the name field
assert not result["name"]

def test_update_id_field(self, group_entity_manager, group_factory):
group_factory()

Expand Down Expand Up @@ -536,19 +525,6 @@ def test_update_group_invalid_field(

assert "new_field" not in result

def test_update_group_empty_field(
self, organization_entity_manager, organization_factory
):
organization = organization_factory()

result = organization_entity_manager.update_entity(
organization["id"], [{"field": "name", "value": ""}]
)

# TODO: this looks like a CKAN bug, we shouldn't be able to nullify
# the name field
assert not result["name"]

def test_update_id_field(self, organization_entity_manager, organization_factory):
organization_factory()

Expand Down

0 comments on commit adb6075

Please sign in to comment.