Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add non-searchable tags #398

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
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
3 changes: 2 additions & 1 deletion invenio_vocabularies/contrib/affiliations/affiliations.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
from invenio_records_resources.records.systemfields import ModelPIDField
from invenio_records_resources.resources.records.headers import etag_headers

from ...services.permissions import PermissionPolicy
from invenio_vocabularies.services.permissions import PermissionPolicy

from .config import AffiliationsSearchOptions, service_components
from .schema import AffiliationSchema

Expand Down
3 changes: 2 additions & 1 deletion invenio_vocabularies/contrib/awards/awards.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
)
from invenio_records_resources.resources.records.headers import etag_headers

from ...services.permissions import PermissionPolicy
from invenio_vocabularies.services.permissions import PermissionPolicy

from ..funders.api import Funder
from ..subjects.api import Subject
from .config import AwardsSearchOptions, service_components
Expand Down
3 changes: 2 additions & 1 deletion invenio_vocabularies/contrib/funders/funders.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
from invenio_records_resources.records.systemfields import ModelPIDField
from invenio_records_resources.resources.records.headers import etag_headers

from ...services.permissions import PermissionPolicy
from invenio_vocabularies.services.permissions import PermissionPolicy

from .config import FundersSearchOptions, service_components
from .schema import FunderSchema
from .serializer import FunderL10NItemSchema
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,41 @@
"description": "Identifier of the name scheme.",
"$ref": "local://definitions-v1.0.0.json#/identifier"
},
"name": {"type": "string"},
"given_name": {"type": "string"},
"family_name": {"type": "string"},
"name": {
"type": "string"
},
"given_name": {
"type": "string"
},
"family_name": {
"type": "string"
},
"identifiers": {
"description": "Identifiers for the person.",
"type": "array",
"items": {"$ref": "local://definitions-v1.0.0.json#/identifiers_with_scheme"},
"items": {
"$ref": "local://definitions-v1.0.0.json#/identifiers_with_scheme"
},
"uniqueItems": true
},
"props": {
"type": "object",
"patternProperties": {
"^.*$": {
"oneOf": [
{
"type": "string"
},
{
"type": "boolean"
},
{
"type": "array"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is array wanted and maybe copied from somewhere else? It might be difficult to have good search queries with arrays.

}
]
}
}
},
"affiliations": {
"description": "Affiliations of the person.",
"type": "array",
Expand All @@ -42,4 +68,4 @@
"uniqueItems": true
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@
"type": "keyword"
}
}
},
"props": {
"type": "object",
"dynamic": "true"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@
"type": "keyword"
}
}
},
"props": {
"type": "object",
"dynamic": "true"
}
}
}
Expand Down
20 changes: 16 additions & 4 deletions invenio_vocabularies/contrib/names/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,23 @@

from invenio_records_permissions.generators import AuthenticatedUser, SystemProcess

from ...services.permissions import PermissionPolicy
from invenio_vocabularies.services.generators import IfTags
from invenio_vocabularies.services.permissions import PermissionPolicy


class NamesPermissionPolicy(PermissionPolicy):
"""Permission policy."""
"""Names permission policy.

can_search = [SystemProcess(), AuthenticatedUser()]
can_read = [SystemProcess(), AuthenticatedUser()]
Names endpoints are protected, only authenticated users can access them.
"""

can_search = [
SystemProcess(),
IfTags(exclude=["unlisted"], only_authenticated=True),
]
Comment on lines +23 to +26
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
can_search = [
SystemProcess(),
IfTags(exclude=["unlisted"], only_authenticated=True),
]
can_search = [
SystemProcess(),
IfTags(exclude=["unlisted"]),
AuthenticatedUser(),
]

minor: wouldn't that be a bit possible in terms of reusing/composing with existing generators? The only_authenticated parameter feels a bit ad-hoc

can_read = [SystemProcess(), IfTags(exclude=["unlisted"], only_authenticated=True)]
# this permission is needed for the /api/vocabularies/ endpoint
can_list_vocabularies = [
SystemProcess(),
IfTags(exclude=["unlisted"], only_authenticated=True),
]
1 change: 1 addition & 0 deletions invenio_vocabularies/contrib/names/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class NameSchema(BaseVocabularySchema, ModePIDFieldVocabularyMixin):
)
)
affiliations = fields.List(fields.Nested(AffiliationRelationSchema))
props = fields.Dict(keys=fields.Str(), values=fields.Raw())

@validates_schema
def validate_names(self, data, **kwargs):
Expand Down
36 changes: 21 additions & 15 deletions invenio_vocabularies/contrib/names/services.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# -*- coding: utf-8 -*-

Check failure on line 1 in invenio_vocabularies/contrib/names/services.py

View workflow job for this annotation

GitHub Actions / Python / Tests (3.9, postgresql14, opensearch2)

Black format check --- /home/runner/work/invenio-vocabularies/invenio-vocabularies/invenio_vocabularies/contrib/names/services.py 2024-10-22 06:51:11.352882+00:00 +++ /home/runner/work/invenio-vocabularies/invenio-vocabularies/invenio_vocabularies/contrib/names/services.py 2024-10-22 06:56:18.138259+00:00 @@ -39,11 +39,11 @@ # the loading process needs to make sure of that if many: results = self._read_many(identity, search_query) else: results = self._read_many(identity, search_query, max_records=1) - + # cant use the results_item because it returns dicts intead of records total = results.hits.total["value"] if total == 0: # Not a PID but trated as such raise PIDDoesNotExistError(pid_type=id_type, pid_value=id_)

Check failure on line 1 in invenio_vocabularies/contrib/names/services.py

View workflow job for this annotation

GitHub Actions / Python / Tests (3.12, postgresql14, opensearch2)

Black format check --- /home/runner/work/invenio-vocabularies/invenio-vocabularies/invenio_vocabularies/contrib/names/services.py 2024-10-22 06:51:11.071871+00:00 +++ /home/runner/work/invenio-vocabularies/invenio-vocabularies/invenio_vocabularies/contrib/names/services.py 2024-10-22 06:57:26.897152+00:00 @@ -39,11 +39,11 @@ # the loading process needs to make sure of that if many: results = self._read_many(identity, search_query) else: results = self._read_many(identity, search_query, max_records=1) - + # cant use the results_item because it returns dicts intead of records total = results.hits.total["value"] if total == 0: # Not a PID but trated as such raise PIDDoesNotExistError(pid_type=id_type, pid_value=id_)
#
# Copyright (C) 2021 CERN.
#
Expand All @@ -19,11 +19,12 @@
class NamesService(record_type.service_cls):
"""Name service."""

def resolve(self, identity, id_, id_type):
def resolve(self, identity, id_, id_type, many=False):
"""Get the record with a given identifier.

This method assumes that the are no duplicates in the system
(i.e. only one name record can have a pair of identifier:scheme).
param id_: The identifier value.
param id_type: The identifier type.
param many: If True, return a list of records.
"""
search_query = dsl.Q(
"bool",
Expand All @@ -36,20 +37,25 @@

# max_records = 1, we assume there cannot be duplicates
# the loading process needs to make sure of that
results = self._read_many(identity, search_query, max_records=1)
if many:
results = self._read_many(identity, search_query)
else:
results = self._read_many(identity, search_query, max_records=1)

# cant use the results_item because it returns dicts intead of records
total = results.hits.total["value"]
if total == 0:
# Not a PID but trated as such
raise PIDDoesNotExistError(pid_type=id_type, pid_value=id_)

# (0 < #hits <= max_records) = 1
record = self.record_cls.loads(results[0].to_dict())
self.require_permission(identity, "read", record=record)

return self.result_item(
self,
identity,
record,
links_tpl=self.links_item_tpl,
)
if many:
return self.result_list(self, identity, results)
Copy link
Contributor

Choose a reason for hiding this comment

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

is there permission needed to read many? I had this thought seeing the permission required below

else:
record = self.record_cls.loads(results[0].to_dict())
self.require_permission(identity, "read", record=record)

return self.result_item(
self,
identity,
record,
links_tpl=self.links_item_tpl,
)
49 changes: 49 additions & 0 deletions invenio_vocabularies/services/generators.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# -*- coding: utf-8 -*-
#
# Copyright (C) 2024 CERN.
#
# Invenio-Vocabularies is free software; you can redistribute it and/or
# modify it under the terms of the MIT License; see LICENSE file for more
# details.
#

"""Vocabulary generators."""

from invenio_access import any_user, authenticated_user
from invenio_records_permissions.generators import Generator
from invenio_search.engine import dsl


class IfTags(Generator):
"""Generator to filter based on tags.

This generator will filter records based on the tags field.
Optionally, it can be configured to only allow authenticated users.
"""

def __init__(self, include=None, exclude=None, only_authenticated=False):
"""Constructor."""
self.include = include or []
self.exclude = exclude or []
self.only_authenticated = only_authenticated

def needs(self, **kwargs):
"""Enabling Needs."""
return [authenticated_user] if self.only_authenticated else [any_user]

def query_filter(self, **kwargs):
"""Search based on configured tags."""
must_clauses = []
must_not_clauses = []

if self.include:
must_clauses.append(dsl.Q("terms", tags=self.include))

if self.exclude:
must_not_clauses.append(dsl.Q("terms", tags=self.exclude))

return dsl.Q(
"bool",
must=must_clauses,
must_not=must_not_clauses,
)
10 changes: 6 additions & 4 deletions invenio_vocabularies/services/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,19 @@
"""Vocabulary permissions."""

from invenio_records_permissions import RecordPermissionPolicy
from invenio_records_permissions.generators import AnyUser, SystemProcess
from invenio_records_permissions.generators import SystemProcess

from invenio_vocabularies.services.generators import IfTags


class PermissionPolicy(RecordPermissionPolicy):
"""Permission policy."""

can_search = [SystemProcess(), AnyUser()]
can_read = [SystemProcess(), AnyUser()]
can_search = [SystemProcess(), IfTags(exclude=["unlisted"])]
can_read = [SystemProcess(), IfTags(exclude=["unlisted"])]
can_create = [SystemProcess()]
can_update = [SystemProcess()]
can_delete = [SystemProcess()]
can_manage = [SystemProcess()]
# this permission is needed for the /api/vocabularies/ endpoint
can_list_vocabularies = [SystemProcess(), AnyUser()]
can_list_vocabularies = [SystemProcess(), IfTags(exclude=["unlisted"])]
26 changes: 25 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@
from flask_principal import Identity, Need, UserNeed
from flask_security import login_user
from flask_security.utils import hash_password
from invenio_access.permissions import ActionUsers, any_user, system_process
from invenio_access.permissions import (
ActionUsers,
any_user,
superuser_access,
system_process,
)
from invenio_access.proxies import current_access
from invenio_accounts.proxies import current_datastore
from invenio_accounts.testutils import login_user_via_session
Expand Down Expand Up @@ -113,6 +118,17 @@ def identity():
return i


@pytest.fixture(scope="module")
def superuser_identity():
"""Super user identity to interact with the services."""
i = Identity(2)
i.provides.add(UserNeed(2))
i.provides.add(any_user)
i.provides.add(system_process)
i.provides.add(superuser_access)
return i


@pytest.fixture(scope="module")
def service(app):
"""Vocabularies service object."""
Expand Down Expand Up @@ -151,6 +167,14 @@ def lang_data2(lang_data):
return data


@pytest.fixture()
def non_searchable_lang_data(lang_data):
"""Example data for testing unlisted cases."""
data = dict(lang_data)
data["tags"] = ["unlisted", "recommended"]
return data


@pytest.fixture()
def example_record(db, identity, service, example_data):
"""Example record."""
Expand Down
29 changes: 29 additions & 0 deletions tests/contrib/names/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@
"""

import pytest
from invenio_indexer.api import RecordIndexer
from invenio_records_resources.proxies import current_service_registry

from invenio_vocabularies.contrib.names.api import Name


@pytest.fixture(scope="module")
def service():
Expand Down Expand Up @@ -56,3 +59,29 @@ def name_full_data():
],
"affiliations": [{"id": "cern"}, {"name": "CustomORG"}],
}


@pytest.fixture(scope="function")
def non_searchable_name_data():
"""Full name data."""
return {
"id": "0000-0001-8135-3489",
"name": "Doe, John",
"given_name": "John",
"family_name": "Doe",
"identifiers": [
{"identifier": "0000-0001-8135-3489", "scheme": "orcid"},
{"identifier": "gnd:4079154-3", "scheme": "gnd"},
],
"affiliations": [{"id": "cern"}, {"name": "CustomORG"}],
"tags": ["unlisted"],
}


@pytest.fixture(scope="module")
def indexer():
"""Indexer instance with correct Record class."""
return RecordIndexer(
record_cls=Name,
record_to_index=lambda r: (r.__class__.index._name, "_doc"),
)
Loading
Loading