Skip to content

Commit 85c212d

Browse files
authored
fix: ensure TOML serialization in show() does not break it (#75)
1 parent df60cc9 commit 85c212d

File tree

3 files changed

+67
-3
lines changed

3 files changed

+67
-3
lines changed

Project.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name = "DataSets"
22
uuid = "c9661210-8a83-48f0-b833-72e62abce419"
33
authors = ["Chris Foster <[email protected]> and contributors"]
4-
version = "0.2.12"
4+
version = "0.2.13"
55

66
[deps]
77
AbstractTrees = "1520ce14-60c1-5f80-bbc7-55ef81b5835c"

src/DataSets.jl

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,21 @@ function Base.show(io::IO, d::DataSet)
162162
end
163163

164164
function Base.show(io::IO, ::MIME"text/plain", d::DataSet)
165-
TOML.print(io, d.conf)
165+
# When we write the DataSet in the show() method above, we just dump the
166+
# underlying dictionary as TOML. However, not all Julia values can be
167+
# serialized, so we do a best-effort sanitization of the dictionary to
168+
# ensure it is serializable.
169+
try
170+
TOML.print(io, d.conf) do _
171+
"<unserializable>"
172+
end
173+
catch e
174+
@debug "Failed to serialize DataSet to TOML" exception = (e, catch_backtrace())
175+
print(io, "\n... <unserializable>")
176+
print(io, "\nSet JULIA_DEBUG=DataSets to see the error")
177+
end
166178
end
167179

168-
169180
#-------------------------------------------------------------------------------
170181
"""
171182
Subtypes of `AbstractDataProject` have the interface

test/runtests.jl

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ using DataSets
44
using Test
55
using UUIDs
66
using ResourceContexts
7+
using TOML
78

89
using DataSets: FileSystemRoot
910

@@ -45,6 +46,58 @@ end
4546

4647
ds = dataset(proj, "a_text_file")
4748
@test ds.uuid == UUID("b498f769-a7f6-4f67-8d74-40b770398f26")
49+
50+
# Exercise the show() methods
51+
let s = sprint(show, ds)
52+
@test occursin("a_text_file", s)
53+
@test occursin("b498f769-a7f6-4f67-8d74-40b770398f26", s)
54+
end
55+
let s = sprint(show, "text/plain", ds)
56+
parsed = TOML.parse(s)
57+
@test parsed isa Dict
58+
@test parsed["name"] == "a_text_file"
59+
@test parsed["uuid"] == "b498f769-a7f6-4f67-8d74-40b770398f26"
60+
end
61+
62+
# Test show() methods when there are values that are not serializable
63+
# into TOML in the Dict
64+
config["datasets"][1]["foo"] = nothing
65+
config["datasets"][1]["bar"] = [1, 2, nothing, Dict("x" => nothing, "y" => "y")]
66+
config["datasets"][1]["baz"] = Dict(
67+
"x" => nothing, "y" => "y",
68+
)
69+
proj = DataSets.load_project(config)
70+
ds = dataset(proj, "a_text_file")
71+
let s = sprint(show, "text/plain", ds)
72+
# It looks like that in Julia <= 1.8.0, the TOML.print(f, ...) variant
73+
# for arbitrary types does not actually work, since it's missing the fallback
74+
# implementation and has other bugs, depending on the Julia version.
75+
#
76+
# So the `show()`-ed TOML will not parse again. But since we have a try-catch anyway,
77+
# we don't care too much, so we just run a simplified test in that case.
78+
if VERSION >= v"1.9.0"
79+
parsed = TOML.parse(s)
80+
@test parsed isa Dict
81+
@test parsed["name"] == "a_text_file"
82+
@test parsed["uuid"] == "b498f769-a7f6-4f67-8d74-40b770398f26"
83+
@test parsed["foo"] == "<unserializable>"
84+
@test parsed["bar"] == [1, 2, "<unserializable>", Dict("x" => "<unserializable>", "y" => "y")]
85+
@test parsed["baz"] == Dict("x" => "<unserializable>", "y" => "y")
86+
else
87+
@test occursin("<unserializable>", s)
88+
end
89+
end
90+
91+
# Also test bad keys
92+
config["datasets"][1]["keys"] = Dict(
93+
nothing => 123,
94+
1234 => "bad",
95+
)
96+
proj = DataSets.load_project(config)
97+
ds = dataset(proj, "a_text_file")
98+
let s = sprint(show, "text/plain", ds)
99+
endswith(s, "\n... <unserializable>\nSet JULIA_DEBUG=DataSets to see the error")
100+
end
48101
end
49102

50103
@testset "open() for DataSet" begin

0 commit comments

Comments
 (0)