From 21ba8a38c57fa91b05a70717ded6573a7c9bd232 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 29 May 2024 14:24:12 +0200 Subject: [PATCH 1/9] status definitions for RCJK: add a failing test first --- tests/test_font.py | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/test_font.py b/tests/test_font.py index 62e7e72..41e3199 100644 --- a/tests/test_font.py +++ b/tests/test_font.py @@ -20,7 +20,7 @@ unstructure, ) -from fontra_rcjk.base import makeSafeLayerName +from fontra_rcjk.base import makeSafeLayerName, standardCustomDataItems dataDir = pathlib.Path(__file__).resolve().parent / "data" @@ -1086,3 +1086,27 @@ async def test_read_write_glyph_customData(writableTestFont): async with contextlib.aclosing(reopenedFont): reopenedGlyph = await reopenedFont.getGlyph(glyphName) assert glyph == reopenedGlyph + + +async def test_statusFieldDefinitions(writableTestFont): + customData = await writableTestFont.getCustomData() + statusDefinitions = customData.get("fontra.sourceStatusFieldDefinitions") + assert ( + standardCustomDataItems.get("fontra.sourceStatusFieldDefinitions") + == statusDefinitions + ) + newStatusDef = { + "color": [0, 0, 0, 1], + "label": "Rejected", + "value": 5, + } + statusDefinitionsCopy = statusDefinitions.copy() + statusDefinitionsCopy.append(newStatusDef) + + await writableTestFont.putCustomData( + {"fontra.sourceStatusFieldDefinitions": statusDefinitionsCopy} + ) + newCustomData = await writableTestFont.getCustomData() + newStatusDefinitions = newCustomData.get("fontra.sourceStatusFieldDefinitions") + + assert newStatusDefinitions[5] == newStatusDef From 16aa2a3345052e197414996d553447fb03ae5b33 Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 29 May 2024 14:33:03 +0200 Subject: [PATCH 2/9] Fix dict replacement if customData has key already --- src/fontra_rcjk/backend_fs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fontra_rcjk/backend_fs.py b/src/fontra_rcjk/backend_fs.py index 9d2b713..aa30ab2 100644 --- a/src/fontra_rcjk/backend_fs.py +++ b/src/fontra_rcjk/backend_fs.py @@ -261,7 +261,7 @@ async def getCustomData(self) -> dict[str, Any]: customDataPath = self.path / FONTLIB_FILENAME if customDataPath.is_file(): customData = json.loads(customDataPath.read_text(encoding="utf-8")) - return customData | standardCustomDataItems + return standardCustomDataItems | customData async def putCustomData(self, customData: dict[str, Any]) -> None: customDataPath = self.path / FONTLIB_FILENAME From dc66c2606a4af72dad11a00c2b334b14420dcb3c Mon Sep 17 00:00:00 2001 From: Olli Meier Date: Wed, 29 May 2024 16:35:28 +0200 Subject: [PATCH 3/9] Update backend_mysql.py in preparation for status definitions --- src/fontra_rcjk/backend_mysql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fontra_rcjk/backend_mysql.py b/src/fontra_rcjk/backend_mysql.py index 400e9f7..6181903 100644 --- a/src/fontra_rcjk/backend_mysql.py +++ b/src/fontra_rcjk/backend_mysql.py @@ -131,7 +131,7 @@ async def taskFunc(): "features", "" ) self._tempFontItemsCache["customData"] = ( - font_data["data"].get("fontlib", {}) | standardCustomDataItems + standardCustomDataItems | font_data["data"].get("fontlib", {}) ) self._tempFontItemsCache.updateTimeOut() del self._getMiscFontItemsTask From 2a6c4c5bd8ad09fe37313d8e918119e8a0c8541a Mon Sep 17 00:00:00 2001 From: Just van Rossum Date: Wed, 29 May 2024 19:31:04 +0200 Subject: [PATCH 4/9] Make test fail, by mutating the returned data, and comparing the actual written json --- tests/test_font.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/test_font.py b/tests/test_font.py index 41e3199..b47dd44 100644 --- a/tests/test_font.py +++ b/tests/test_font.py @@ -1,4 +1,5 @@ import contextlib +import json import pathlib import shutil from importlib.metadata import entry_points @@ -1100,13 +1101,19 @@ async def test_statusFieldDefinitions(writableTestFont): "label": "Rejected", "value": 5, } - statusDefinitionsCopy = statusDefinitions.copy() - statusDefinitionsCopy.append(newStatusDef) + statusDefinitions.append(newStatusDef) + + editedCustomData = customData | { + "fontra.sourceStatusFieldDefinitions": statusDefinitions + } + await writableTestFont.putCustomData(editedCustomData) - await writableTestFont.putCustomData( - {"fontra.sourceStatusFieldDefinitions": statusDefinitionsCopy} - ) newCustomData = await writableTestFont.getCustomData() newStatusDefinitions = newCustomData.get("fontra.sourceStatusFieldDefinitions") assert newStatusDefinitions[5] == newStatusDef + + fontLibPath = writableTestFont.path / "fontLib.json" + fontLib = json.loads(fontLibPath.read_text()) + + assert editedCustomData == fontLib From c8e34c4e5c6c3e554e73a903de870f857424576e Mon Sep 17 00:00:00 2001 From: Just van Rossum Date: Wed, 29 May 2024 19:31:36 +0200 Subject: [PATCH 5/9] Change color tuples into lists, so we can round-trip via json without change --- src/fontra_rcjk/base.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/fontra_rcjk/base.py b/src/fontra_rcjk/base.py index edc413b..a3b186e 100644 --- a/src/fontra_rcjk/base.py +++ b/src/fontra_rcjk/base.py @@ -521,28 +521,28 @@ def makeSafeLayerName(layerName): "fontra.sourceStatusFieldDefinitions": [ { "label": "In progress", - "color": (1.0, 0.0, 0.0, 1.0), + "color": [1.0, 0.0, 0.0, 1.0], "value": 0, "isDefault": True, }, { "label": "Checking-1", - "color": (1.0, 0.5, 0.0, 1.0), + "color": [1.0, 0.5, 0.0, 1.0], "value": 1, }, { "label": "Checking-2", - "color": (1.0, 1.0, 0.0, 1.0), + "color": [1.0, 1.0, 0.0, 1.0], "value": 2, }, { "label": "Checking-3", - "color": (0.0, 0.5, 1.0, 1.0), + "color": [0.0, 0.5, 1.0, 1.0], "value": 3, }, { "label": "Validated", - "color": (0.0, 1.0, 0.5, 1.0), + "color": [0.0, 1.0, 0.5, 1.0], "value": 4, }, ] From b2fa741590bc623db62c54c5d29533e3cf861a02 Mon Sep 17 00:00:00 2001 From: Just van Rossum Date: Wed, 29 May 2024 19:32:23 +0200 Subject: [PATCH 6/9] Fix mutating bug by deepcopying the standard items --- src/fontra_rcjk/backend_fs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fontra_rcjk/backend_fs.py b/src/fontra_rcjk/backend_fs.py index aa30ab2..28c1e4b 100644 --- a/src/fontra_rcjk/backend_fs.py +++ b/src/fontra_rcjk/backend_fs.py @@ -261,7 +261,7 @@ async def getCustomData(self) -> dict[str, Any]: customDataPath = self.path / FONTLIB_FILENAME if customDataPath.is_file(): customData = json.loads(customDataPath.read_text(encoding="utf-8")) - return standardCustomDataItems | customData + return deepcopy(standardCustomDataItems) | customData async def putCustomData(self, customData: dict[str, Any]) -> None: customDataPath = self.path / FONTLIB_FILENAME From 727b7bdbd14c005ca95e13488f299d9622b3cb74 Mon Sep 17 00:00:00 2001 From: Just van Rossum Date: Wed, 29 May 2024 19:32:58 +0200 Subject: [PATCH 7/9] deepcopy() the customData: our own copy should not be modified --- src/fontra_rcjk/backend_mysql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fontra_rcjk/backend_mysql.py b/src/fontra_rcjk/backend_mysql.py index 6181903..89f7d7e 100644 --- a/src/fontra_rcjk/backend_mysql.py +++ b/src/fontra_rcjk/backend_mysql.py @@ -220,7 +220,7 @@ async def getCustomData(self) -> dict[str, Any]: if customData is None: await self._getMiscFontItems() customData = self._tempFontItemsCache["customData"] - return customData + return deepcopy(customData) async def putCustomData(self, customData: dict[str, Any]) -> None: await self._getMiscFontItems() From 9b2b99e2cc9408bab31c2eadd9bbf30021937947 Mon Sep 17 00:00:00 2001 From: Just van Rossum Date: Wed, 29 May 2024 19:34:05 +0200 Subject: [PATCH 8/9] Don't smart-filter customData on write --- src/fontra_rcjk/backend_fs.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/fontra_rcjk/backend_fs.py b/src/fontra_rcjk/backend_fs.py index 28c1e4b..18b2423 100644 --- a/src/fontra_rcjk/backend_fs.py +++ b/src/fontra_rcjk/backend_fs.py @@ -265,11 +265,6 @@ async def getCustomData(self) -> dict[str, Any]: async def putCustomData(self, customData: dict[str, Any]) -> None: customDataPath = self.path / FONTLIB_FILENAME - customData = { - k: v - for k, v in customData.items() - if k not in standardCustomDataItems or standardCustomDataItems[k] != v - } customDataPath.write_text(json.dumps(customData, indent=2), encoding="utf-8") async def watchExternalChanges( From ede1a9fcf2054d9accecedaf129920dd13de3c0d Mon Sep 17 00:00:00 2001 From: Just van Rossum Date: Wed, 29 May 2024 19:36:00 +0200 Subject: [PATCH 9/9] Don't use .get() when we know the key must exist --- tests/test_font.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_font.py b/tests/test_font.py index b47dd44..e9e64a6 100644 --- a/tests/test_font.py +++ b/tests/test_font.py @@ -1091,9 +1091,9 @@ async def test_read_write_glyph_customData(writableTestFont): async def test_statusFieldDefinitions(writableTestFont): customData = await writableTestFont.getCustomData() - statusDefinitions = customData.get("fontra.sourceStatusFieldDefinitions") + statusDefinitions = customData["fontra.sourceStatusFieldDefinitions"] assert ( - standardCustomDataItems.get("fontra.sourceStatusFieldDefinitions") + standardCustomDataItems["fontra.sourceStatusFieldDefinitions"] == statusDefinitions ) newStatusDef = { @@ -1109,7 +1109,7 @@ async def test_statusFieldDefinitions(writableTestFont): await writableTestFont.putCustomData(editedCustomData) newCustomData = await writableTestFont.getCustomData() - newStatusDefinitions = newCustomData.get("fontra.sourceStatusFieldDefinitions") + newStatusDefinitions = newCustomData["fontra.sourceStatusFieldDefinitions"] assert newStatusDefinitions[5] == newStatusDef