From fe4bb0b49933e5ceb11d9ce6cf1b538a8122886b Mon Sep 17 00:00:00 2001 From: YishaiGlasner Date: Sun, 24 Dec 2023 11:45:18 +0200 Subject: [PATCH 1/8] doc(api-v3): doc strings for Version attributes. --- sefaria/model/text.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sefaria/model/text.py b/sefaria/model/text.py index 22c2e19199..26e486144b 100644 --- a/sefaria/model/text.py +++ b/sefaria/model/text.py @@ -1334,12 +1334,12 @@ class Version(AbstractTextRecord, abst.AbstractMongoRecord, AbstractSchemaConten "purchaseInformationImage", "purchaseInformationURL", "hasManuallyWrappedRefs", # true for texts where refs were manually wrapped in a-tags. no need to run linker at run-time. - "actualLanguage", - 'languageFamilyName', - "isBaseText", - 'isSource', - 'isPrimary', - 'direction', # 'rtl' or 'ltr' + "actualLanguage", # ISO language code + 'languageFamilyName', # full name of the language, but without specificity (for Judeo Arabic actualLanguage=jrb, languageFamilyName=arabic + "isBaseText", # should be deprecated (needs some changes on client side) + 'isSource', # bool, True if this version is not a translation + 'isPrimary', # bool, True if we see it as a primary version (usually equals to isSource, but Hebrew Kuzarif or example is primary but not source) + 'direction', # 'rtl' or 'ltr' ] def __str__(self): From 4e482c0708818c516d21d7526a4eda1f3210df74 Mon Sep 17 00:00:00 2001 From: YishaiGlasner Date: Sun, 24 Dec 2023 11:51:00 +0200 Subject: [PATCH 2/8] fix(api-v3): change 400 to 404 when ref is invalid or empty. --- api/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/views.py b/api/views.py index 982818902c..05e3b6c1d8 100644 --- a/api/views.py +++ b/api/views.py @@ -13,7 +13,7 @@ def dispatch(self, request, *args, **kwargs): try: self.oref = Ref.instantiate_ref_with_legacy_parse_fallback(kwargs['tref']) except Exception as e: - return jsonResponse({'error': getattr(e, 'message', str(e))}, status=400) + return jsonResponse({'error': getattr(e, 'message', str(e))}, status=404) return super().dispatch(request, *args, **kwargs) @staticmethod @@ -43,7 +43,7 @@ def _handle_warnings(self, data): def get(self, request, *args, **kwargs): if self.oref.is_empty() and not self.oref.index_node.is_virtual: - return jsonResponse({'error': f'We have no text for {self.oref}.'}, status=400) + return jsonResponse({'error': f'We have no text for {self.oref}.'}, status=404) versions_params = request.GET.getlist('version', []) if not versions_params: versions_params = ['primary'] From 201d8af85cb53e9e3aebee30373b530d4bd3af41 Mon Sep 17 00:00:00 2001 From: YishaiGlasner Date: Sun, 24 Dec 2023 11:53:50 +0200 Subject: [PATCH 3/8] refactor(api-v3): change TextManger to TextRequestAdapter. --- api/views.py | 4 ++-- sefaria/model/{text_manager.py => text_reuqest_adapter.py} | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) rename sefaria/model/{text_manager.py => text_reuqest_adapter.py} (99%) diff --git a/api/views.py b/api/views.py index 05e3b6c1d8..0e714da047 100644 --- a/api/views.py +++ b/api/views.py @@ -1,5 +1,5 @@ from sefaria.model import * -from sefaria.model.text_manager import TextManager +from sefaria.model.text_reuqest_adapter import TextRequestAdapter from sefaria.client.util import jsonResponse from django.views import View from .api_warnings import * @@ -52,7 +52,7 @@ def get(self, request, *args, **kwargs): return_format = request.GET.get('return_format', 'default') if return_format not in self.RETURN_FORMATS: return jsonResponse({'error': f'return_format should be one of those formats: {self.RETURN_FORMATS}.'}, status=400) - text_manager = TextManager(self.oref, versions_params, fill_in_missing_segments, return_format) + text_manager = TextRequestAdapter(self.oref, versions_params, fill_in_missing_segments, return_format) data = text_manager.get_versions_for_query() data = self._handle_warnings(data) return jsonResponse(data) diff --git a/sefaria/model/text_manager.py b/sefaria/model/text_reuqest_adapter.py similarity index 99% rename from sefaria/model/text_manager.py rename to sefaria/model/text_reuqest_adapter.py index f14a54bac8..e6963cf348 100644 --- a/sefaria/model/text_manager.py +++ b/sefaria/model/text_reuqest_adapter.py @@ -9,7 +9,7 @@ from sefaria.system.exceptions import InputError from sefaria.datatype.jagged_array import JaggedTextArray -class TextManager: +class TextRequestAdapter: ALL = 'all' PRIMARY = 'primary' SOURCE = 'source' From 2943990b858b5eb57776418684d07cbaa2fd481e Mon Sep 17 00:00:00 2001 From: YishaiGlasner Date: Sun, 24 Dec 2023 11:58:02 +0200 Subject: [PATCH 4/8] doc(api-v3): doc string for TextRequestAdapter. --- sefaria/model/text_reuqest_adapter.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sefaria/model/text_reuqest_adapter.py b/sefaria/model/text_reuqest_adapter.py index e6963cf348..1bae459909 100644 --- a/sefaria/model/text_reuqest_adapter.py +++ b/sefaria/model/text_reuqest_adapter.py @@ -10,6 +10,11 @@ from sefaria.datatype.jagged_array import JaggedTextArray class TextRequestAdapter: + """ + This class is used for getting texts for client side (API or SSR) + It takes the same params as the api/v3/text (ref, version_params that are language and versionTitle, fill_in_missing_segments, and return_format + It returns a JSON-like object for an HTTP response. + """ ALL = 'all' PRIMARY = 'primary' SOURCE = 'source' From de5e59debbb065a6c5462275f684795e378708c9 Mon Sep 17 00:00:00 2001 From: YishaiGlasner Date: Sun, 24 Dec 2023 12:05:42 +0200 Subject: [PATCH 5/8] doc(api-v3): doc string for TextRange. --- sefaria/model/text.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sefaria/model/text.py b/sefaria/model/text.py index 26e486144b..56d596000c 100644 --- a/sefaria/model/text.py +++ b/sefaria/model/text.py @@ -1696,6 +1696,11 @@ def __call__(cls, *args, **kwargs): class TextRange: + """ + This class is planned to replace TextChunk, using real language rather than he/en + For now it's used by v3 texts api + It can be used for getting text, but not yet for saving + """ def __init__(self, oref, lang, vtitle, merge_versions=False): if isinstance(oref.index_node, JaggedArrayNode) or isinstance(oref.index_node, DictionaryEntryNode): #text cannot be SchemaNode From 01eb7d1067e4e58c58f4f5a5b30ea18fe2804d81 Mon Sep 17 00:00:00 2001 From: YishaiGlasner Date: Sun, 24 Dec 2023 15:20:33 +0200 Subject: [PATCH 6/8] refactor(api-v3): change versions to be a private param with a public setter. --- sefaria/model/text.py | 25 +++++++++++-------------- sefaria/model/text_reuqest_adapter.py | 4 ++-- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/sefaria/model/text.py b/sefaria/model/text.py index 56d596000c..e72d3728da 100644 --- a/sefaria/model/text.py +++ b/sefaria/model/text.py @@ -1700,9 +1700,10 @@ class TextRange: This class is planned to replace TextChunk, using real language rather than he/en For now it's used by v3 texts api It can be used for getting text, but not yet for saving + The versions param is for better performance when the version(s) were already loaded from mongo """ - def __init__(self, oref, lang, vtitle, merge_versions=False): + def __init__(self, oref, lang, vtitle, merge_versions=False, versions=None): if isinstance(oref.index_node, JaggedArrayNode) or isinstance(oref.index_node, DictionaryEntryNode): #text cannot be SchemaNode self.oref = oref elif oref.has_default_child(): #use default child: @@ -1712,22 +1713,18 @@ def __init__(self, oref, lang, vtitle, merge_versions=False): self.lang = lang self.vtitle = vtitle self.merge_versions = merge_versions - self._versions = [] self._text = None self.sources = None + self._set_versions(versions) - @property - def versions(self): - if self._versions == []: + def _set_versions(self, versions): + if versions: + self._validate_versions(versions) + self._versions = versions + else: condition_query = self.oref.condition_query(self.lang) if self.merge_versions else \ {'title': self.oref.index.title, 'languageFamilyName': self.lang, 'versionTitle': self.vtitle} self._versions = VersionSet(condition_query, proj=self.oref.part_projection()) - return self._versions - - @versions.setter - def versions(self, versions): - self._validate_versions(versions) - self._versions = versions def _validate_versions(self, versions): if not self.merge_versions and len(versions) > 1: @@ -1758,15 +1755,15 @@ def _trim_text(self, text): @property def text(self): if self._text is None: - if self.merge_versions and len(self.versions) > 1: - merged_text, sources = self.versions.merge(self.oref.index_node, prioritized_vtitle=self.vtitle) + if self.merge_versions and len(self._versions) > 1: + merged_text, sources = self._versions.merge(self.oref.index_node, prioritized_vtitle=self.vtitle) self._text = self._trim_text(merged_text) if len(sources) > 1: self.sources = sources elif self.oref.index_node.is_virtual: self._text = self.oref.index_node.get_text() else: - self._text = self._trim_text(self.versions[0].content_node(self.oref.index_node)) #todo if there is no version it will fail + self._text = self._trim_text(self._versions[0].content_node(self.oref.index_node)) #todo if there is no version it will fail return self._text diff --git a/sefaria/model/text_reuqest_adapter.py b/sefaria/model/text_reuqest_adapter.py index 1bae459909..e47e5fd047 100644 --- a/sefaria/model/text_reuqest_adapter.py +++ b/sefaria/model/text_reuqest_adapter.py @@ -43,7 +43,6 @@ def _append_version(self, version): for attr in ['chapter', 'title']: fields.remove(attr) version_details = {f: getattr(version, f, "") for f in fields} - text_range = TextRange(self.oref, version.languageFamilyName, version.versionTitle, self.fill_in_missing_segments) if self.fill_in_missing_segments: # we need a new VersionSet of only the relevant versions for merging. copy should be better than calling for mongo @@ -51,7 +50,8 @@ def _append_version(self, version): relevant_versions.remove(lambda v: v.languageFamilyName != version.languageFamilyName) else: relevant_versions = [version] - text_range.versions = relevant_versions + text_range = TextRange(self.oref, version.languageFamilyName, version.versionTitle, + self.fill_in_missing_segments, relevant_versions) version_details['text'] = text_range.text sources = getattr(text_range, 'sources', None) From fc8ff4c974566baa604fb70962fc9eca7055c066 Mon Sep 17 00:00:00 2001 From: YishaiGlasner Date: Sun, 24 Dec 2023 15:21:11 +0200 Subject: [PATCH 7/8] test(api-v3): fix tests to fit the 400-404 change. --- api/tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/tests.py b/api/tests.py index 76709e1612..c3b0ffefba 100644 --- a/api/tests.py +++ b/api/tests.py @@ -119,7 +119,7 @@ def text_api_virtual_node(self): def test_api_get_text_bad_text(self): response = c.get('/api/v3/texts/Life_of_Pi.13.13') - self.assertEqual(400, response.status_code) + self.assertEqual(404, response.status_code) data = json.loads(response.content) self.assertEqual(data["error"], "Could not find title in reference: Life of Pi.13.13") @@ -135,13 +135,13 @@ def test_api_get_text_too_many_hyphens(self): def test_api_get_text_bad_sections(self): response = c.get('/api/v3/texts/Job.6-X') - self.assertEqual(400, response.status_code) + self.assertEqual(404, response.status_code) data = json.loads(response.content) self.assertEqual(data["error"], "Couldn't understand text sections: 'Job.6-X'.") def test_api_get_text_empty_ref(self): response = c.get("/api/v3/texts/Berakhot.1a") - self.assertEqual(400, response.status_code) + self.assertEqual(404, response.status_code) data = json.loads(response.content) self.assertEqual(data["error"], "We have no text for Berakhot 1a.") From 5f3fa2402e64d28fd5e91e0d810569f02c46d737 Mon Sep 17 00:00:00 2001 From: YishaiGlasner Date: Sun, 24 Dec 2023 16:11:30 +0200 Subject: [PATCH 8/8] refactor(api-v3): change isBaseText on client side to isSource (next step is deprecating it). --- static/js/VersionBlock/VersionBlock.jsx | 2 +- static/js/sefaria/sefaria.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/static/js/VersionBlock/VersionBlock.jsx b/static/js/VersionBlock/VersionBlock.jsx index 4cf6828bba..36a33099f2 100644 --- a/static/js/VersionBlock/VersionBlock.jsx +++ b/static/js/VersionBlock/VersionBlock.jsx @@ -187,7 +187,7 @@ class VersionBlock extends Component { } } makeSelectVersionLanguage(){ - let voc = this.props.version.isBaseText ? 'Version' : "Translation"; + let voc = this.props.version.isSource ? 'Version' : "Translation"; return this.props.isCurrent ? Sefaria._("Current " + voc) : Sefaria._("Select "+ voc); } diff --git a/static/js/sefaria/sefaria.js b/static/js/sefaria/sefaria.js index 16d4e456dd..a39d3b2566 100644 --- a/static/js/sefaria/sefaria.js +++ b/static/js/sefaria/sefaria.js @@ -692,13 +692,13 @@ Sefaria = extend(Sefaria, { }, getTranslations: async function(ref) { /** - * Gets all versions except Hebrew versions that have isBaseText true + * Gets all versions except Hebrew versions that have isSource true * @ref {string} ref * @returns {string: [versions]} Versions by language */ return Sefaria.getVersions(ref).then(result => { let versions = Sefaria.filterVersionsObjByLangs(result, ['he'], false); - let heVersions = Sefaria.filterVersionsArrayByAttr(result?.he || [], {isBaseText: false}); + let heVersions = Sefaria.filterVersionsArrayByAttr(result?.he || [], {isSource: false}); if (heVersions.length) { versions.he = heVersions; }