From a932893edc477a710ec7e72b2159288213ace40e Mon Sep 17 00:00:00 2001 From: Stepland <16676308+Stepland@users.noreply.github.com> Date: Tue, 1 Jun 2021 19:06:53 +0200 Subject: [PATCH] [malody] Dumping does not write placeholder `null` values anymore --- CHANGELOG.md | 4 ++ .../jubeat_analyser/tests/test_utils.py | 21 +++++- jubeatools/formats/malody/dump.py | 10 +-- jubeatools/formats/malody/schema.py | 36 +++++----- .../formats/malody/tests/test_malody.py | 71 ++++++++++++++++--- jubeatools/testutils/test_patterns.py | 14 ++-- 6 files changed, 118 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9780030..efd49cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# v1.2.2 +## Fixed +- [malody] Dumping does not write placeholder `null` values anymore + # v1.2.1 ## Fixed - [malody] Parsing a file with keys that are unused for conversion diff --git a/jubeatools/formats/jubeat_analyser/tests/test_utils.py b/jubeatools/formats/jubeat_analyser/tests/test_utils.py index eb2379a..716c30f 100644 --- a/jubeatools/formats/jubeat_analyser/tests/test_utils.py +++ b/jubeatools/formats/jubeat_analyser/tests/test_utils.py @@ -12,7 +12,26 @@ from jubeatools.testutils.typing import DrawFunc @st.composite def memo_compatible_metadata(draw: DrawFunc) -> song.Metadata: - text_strat = st.text(alphabet=st.characters(min_codepoint=0x20, max_codepoint=0x7E)) + # some ranges that are valid in shift-jis + text_strat = st.text( + alphabet=st.one_of( + *( + st.characters(min_codepoint=a, max_codepoint=b) + for a, b in ( + (0x20, 0x7F), + (0xB6, 0x109), + (0x410, 0x44F), + (0x24D0, 0x24E9), + (0x3041, 0x3096), + (0x309B, 0x30FF), + (0xFA30, 0xFA6A), + (0xFF01, 0xFF3B), + (0xFF3D, 0xFF5D), + (0xFF61, 0xFF9F), + ) + ) + ) + ) metadata: song.Metadata = draw( jbst.metadata(text_strat=text_strat, path_strat=text_strat) ) diff --git a/jubeatools/formats/malody/dump.py b/jubeatools/formats/malody/dump.py index 2d6dac6..cffa553 100644 --- a/jubeatools/formats/malody/dump.py +++ b/jubeatools/formats/malody/dump.py @@ -1,7 +1,7 @@ import time from functools import singledispatch from pathlib import Path -from typing import List, Tuple, Union +from typing import List, Optional, Tuple, Union import simplejson as json @@ -30,7 +30,7 @@ dump_malody = make_dumper_from_chart_file_dumper( def dump_malody_chart( - metadata: song.Metadata, dif: str, chart: song.Chart, timing: song.Timing + metadata: song.Metadata, dif: Optional[str], chart: song.Chart, timing: song.Timing ) -> malody.Chart: meta = dump_metadata(metadata, dif) time = dump_timing(timing) @@ -40,10 +40,10 @@ def dump_malody_chart( return malody.Chart(meta=meta, time=time, note=notes) -def dump_metadata(metadata: song.Metadata, dif: str) -> malody.Metadata: +def dump_metadata(metadata: song.Metadata, dif: Optional[str]) -> malody.Metadata: return malody.Metadata( - cover="", - creator="", + cover=None, + creator=None, background=none_or(str, metadata.cover), version=dif, id=0, diff --git a/jubeatools/formats/malody/schema.py b/jubeatools/formats/malody/schema.py index 3c7c16e..57fb786 100644 --- a/jubeatools/formats/malody/schema.py +++ b/jubeatools/formats/malody/schema.py @@ -1,21 +1,15 @@ from dataclasses import dataclass, field from decimal import Decimal from enum import Enum -from typing import List, Optional, Tuple, Union +from typing import Any, List, Optional, Tuple, Union -from marshmallow import EXCLUDE +from marshmallow import EXCLUDE, Schema, post_dump from marshmallow.validate import Range from marshmallow_dataclass import NewType, class_schema -class Ordered: - class Meta: - ordered = True - unknown = EXCLUDE - - @dataclass -class SongInfo(Ordered): +class SongInfo: title: Optional[str] artist: Optional[str] id: Optional[int] @@ -32,7 +26,7 @@ class Mode(int, Enum): @dataclass -class Metadata(Ordered): +class Metadata: cover: Optional[str] # path to album art ? creator: Optional[str] # Chart author background: Optional[str] # path to background image @@ -51,7 +45,7 @@ StrictlyPositiveDecimal = NewType( @dataclass -class BPMEvent(Ordered): +class BPMEvent: beat: BeatTime bpm: StrictlyPositiveDecimal @@ -60,13 +54,13 @@ ButtonIndex = NewType("ButtonIndex", int, validate=Range(min=0, max=15)) @dataclass -class TapNote(Ordered): +class TapNote: beat: BeatTime index: ButtonIndex @dataclass -class LongNote(Ordered): +class LongNote: beat: BeatTime index: ButtonIndex endbeat: BeatTime @@ -74,7 +68,7 @@ class LongNote(Ordered): @dataclass -class Sound(Ordered): +class Sound: """Used both for the background music and keysounds""" beat: BeatTime @@ -95,10 +89,20 @@ Event = Union[Sound, LongNote, TapNote] @dataclass -class Chart(Ordered): +class Chart: meta: Metadata time: List[BPMEvent] = field(default_factory=list) note: List[Event] = field(default_factory=list) -CHART_SCHEMA = class_schema(Chart)() +class BaseSchema(Schema): + class Meta: + ordered = True + unknown = EXCLUDE + + @post_dump + def remove_none_values(self, data: dict, **kwargs: Any) -> dict: + return {key: value for key, value in data.items() if value is not None} + + +CHART_SCHEMA = class_schema(Chart, base_schema=BaseSchema)() diff --git a/jubeatools/formats/malody/tests/test_malody.py b/jubeatools/formats/malody/tests/test_malody.py index b41395d..76c2115 100644 --- a/jubeatools/formats/malody/tests/test_malody.py +++ b/jubeatools/formats/malody/tests/test_malody.py @@ -1,32 +1,85 @@ +from dataclasses import fields from decimal import Decimal +from typing import Optional +import simplejson as json from hypothesis import given from hypothesis import strategies as st from jubeatools import song from jubeatools.formats import Format from jubeatools.formats.konami.testutils import open_temp_dir +from jubeatools.formats.malody import schema as malody +from jubeatools.formats.malody.dump import dump_malody_chart from jubeatools.testutils import strategies as jbst from jubeatools.testutils.test_patterns import dump_and_load_then_compare from jubeatools.testutils.typing import DrawFunc @st.composite -def malody_compatible_song(draw: DrawFunc) -> song.Song: - """Malody files only hold one chart and have limited metadata""" - diff = draw(st.sampled_from(list(song.Difficulty))).value - chart = draw(jbst.chart(level_strat=st.just(Decimal(0)))) - metadata = draw(jbst.metadata()) +def difficulty(draw: DrawFunc) -> str: + d: song.Difficulty = draw(st.sampled_from(list(song.Difficulty))) + return d.value + + +@st.composite +def chart(draw: DrawFunc) -> song.Chart: + c: song.Chart = draw(jbst.chart(level_strat=st.just(Decimal(0)))) + return c + + +@st.composite +def metadata(draw: DrawFunc) -> song.Metadata: + metadata: song.Metadata = draw(jbst.metadata()) metadata.preview = None metadata.preview_file = None - return song.Song(metadata=metadata, charts={diff: chart}) + return metadata -@given(malody_compatible_song()) -def test_that_full_chart_roundtrips(song: song.Song) -> None: +@st.composite +def malody_song(draw: DrawFunc) -> song.Song: + """Malody files only hold one chart and have limited metadata""" + diff = draw(difficulty()) + chart_ = draw(chart()) + metadata_ = draw(metadata()) + return song.Song(metadata=metadata_, charts={diff: chart_}) + + +@given(malody_song()) +def test_that_full_chart_roundtrips(s: song.Song) -> None: dump_and_load_then_compare( Format.MALODY, - song, + s, temp_path=open_temp_dir(), bytes_decoder=lambda b: b.decode("utf-8"), ) + + +@given(chart(), metadata(), st.one_of(st.none(), difficulty())) +def test_that_none_values_in_metadata_dont_appear_in_dumped_json( + chart: song.Chart, + metadata: song.Metadata, + dif: Optional[str], +) -> None: + assert chart.timing is not None + malody_chart = dump_malody_chart(metadata, dif, chart, chart.timing) + json_chart = malody.CHART_SCHEMA.dump(malody_chart) + assert all(value is not None for value in json_chart["meta"].values()) + + +@given(malody_song()) +def test_that_field_are_ordered(s: song.Song) -> None: + dif, chart = next(iter(s.charts.items())) + assert chart.timing is not None + malody_chart = dump_malody_chart(s.metadata, dif, chart, chart.timing) + json_chart = malody.CHART_SCHEMA.dump(malody_chart) + text_chart = json.dumps(json_chart, indent=4, use_decimal=True) + reparsed_chart = json.loads( + text_chart, + ) + # dict is ordered in 3.8 ... right ? + order_in_file = list(reparsed_chart["meta"].keys()) + order_in_definition = list( + f.name for f in fields(malody.Metadata) if f.name in reparsed_chart["meta"] + ) + assert order_in_file == order_in_definition diff --git a/jubeatools/testutils/test_patterns.py b/jubeatools/testutils/test_patterns.py index ac5edc4..f91c2ca 100644 --- a/jubeatools/testutils/test_patterns.py +++ b/jubeatools/testutils/test_patterns.py @@ -21,11 +21,11 @@ def dump_and_load_then_compare( dump_options = dump_options or {} loader = LOADERS[format_] dumper = DUMPERS[format_] - with temp_path as path: - files = dumper(song, path, **dump_options) - for path, bytes_ in files.items(): - path.write_bytes(bytes_) - note(f"Wrote to {path} :\n{bytes_decoder(bytes_)}") - assert guess_format(path) == format_ - recovered_song = loader(path, **load_options) + with temp_path as folder_path: + files = dumper(song, folder_path, **dump_options) + for file_path, bytes_ in files.items(): + file_path.write_bytes(bytes_) + note(f"Wrote to {file_path} :\n{bytes_decoder(bytes_)}") + assert guess_format(file_path) == format_ + recovered_song = loader(folder_path, **load_options) assert recovered_song == song