From 28b2185ee17ea8ead0d0ffd0dea03ebd5fde8060 Mon Sep 17 00:00:00 2001
From: JonasIsensee <jonas.isensee@web.de>
Date: Thu, 4 Jan 2024 09:40:59 +0100
Subject: [PATCH] fix reconstruction with unknown type param (#521)

* fix reconstruction with unknown type param

* fix and test
---
 src/data/reconstructing_datatypes.jl | 24 ++++++++++++------------
 test/Artifacts.toml                  |  6 +++---
 test/test_files.jl                   | 12 +++++++++++-
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/src/data/reconstructing_datatypes.jl b/src/data/reconstructing_datatypes.jl
index b9dda795..1375c140 100644
--- a/src/data/reconstructing_datatypes.jl
+++ b/src/data/reconstructing_datatypes.jl
@@ -368,7 +368,7 @@ function types_from_refs(f::JLDFile, ptr::Ptr)
             nulldt = CommittedDatatype(UNDEFINED_ADDRESS, 0)
             cdt = get(f.datatype_locations, ref, nulldt)
             res = cdt !== nulldt ? (typeof(jltype(f, cdt)::ReadRepresentation)::DataType).parameters[1] : load_dataset(f, ref)
-            unknown_params = unknown_params || isunknowntype(res)
+            unknown_params = unknown_params || isunknowntype(res) || isreconstructed(res)
             res
         end for ref in refs]
         return params, unknown_params
@@ -402,9 +402,9 @@ function jlconvert(rr::ReadRepresentation{T,DataTypeODR()},
         m isa Upgrade && return m
     else
         m = _resolve_type(rr, f, ptr, header_offset, mypath, hasparams, hasparams ? params : nothing)
-        isunknowntype(m) && return m
     end
-
+    isunknowntype(m) && return m
+    unknown_params && return UnknownType{m, Tuple{params...}}
     if hasparams
         try
             m = m{params...}
@@ -557,7 +557,7 @@ function constructrr(f::JLDFile, unk::Type{UnknownType{T,P}}, dt::CompoundDataty
     attrs::Vector{ReadAttribute}) where {T,P}
     field_datatypes = read_field_datatypes(f, attrs)
     if T isa DataType
-        if unk.name == Tuple
+        if T === Tuple
             # For a tuple with unknown fields, we should reconstruct the fields
             rodr = reconstruct_odr(f, dt, field_datatypes)
             # This is a "pseudo-RR" since the tuple is not fully parametrized, but
@@ -569,15 +569,15 @@ function constructrr(f::JLDFile, unk::Type{UnknownType{T,P}}, dt::CompoundDataty
             
         end
     elseif T isa UnionAll
-        body = behead(unk.name)
-        if length(body.parameters) != length(unk.parameters)
-            @warn("read type $(typestring(unk)) has a different number of parameters from type " *
-                "$(unk.name) in workspace; reconstructing")
-            reconstruct_compound(f, typestring(unk), dt, field_datatypes)
+        body = behead(T)
+        if length(body.parameters) != length(P.parameters)
+            @warn("read type $(typestring(T)) has a different number of parameters from type " *
+                "$(T) in workspace; reconstructing")
+            reconstruct_compound(f, typestring(T), dt, field_datatypes)
         else
             params = [P.parameters...,]
             for i = 1:length(params)
-                if isa(params[i], UnknownType)
+                if isunknowntype(params[i])
                     param = body.parameters[i]::TypeVar
                     params[i] = param.ub
                 end
@@ -585,8 +585,8 @@ function constructrr(f::JLDFile, unk::Type{UnknownType{T,P}}, dt::CompoundDataty
 
             # Try to construct the rr for the relaxed type. On failure, fall back to
             # reconstruct_compound.`
-            try
-                T2 = unk.name{params...}
+            T2 = try
+                T{params...}
             catch err
                 @warn("type parameters for $(typestring(unk)) do not match type $(T) in workspace; reconstructing")
                 return reconstruct_compound(f, shorttypestring(unk), dt, field_datatypes)
diff --git a/test/Artifacts.toml b/test/Artifacts.toml
index 1dc2eda7..48f57886 100644
--- a/test/Artifacts.toml
+++ b/test/Artifacts.toml
@@ -1,7 +1,7 @@
 [testfiles]
-git-tree-sha1 = "496c72594b20fe977506afc3c7af39e8dedae53d"
+git-tree-sha1 = "8d34be7b8603854dca03eb0330f214a9194de75a"
 lazy = true
 
     [[testfiles.download]]
-    url = "https://github.com/JonasIsensee/JLD2TestFiles/archive/refs/tags/v0.1.0.tar.gz"
-    sha256 = "e2e00e745a840b5a76cdf8b8bf17c78d135985c86397a8e53e0fd678ab42f4f5"
\ No newline at end of file
+    sha256 = "f91805f612a00178ca68d5ecc895af09c231d1ca949c7ef0157cb9730b3a84aa"
+    url = "https://github.com/JonasIsensee/JLD2TestFiles/archive/refs/tags/v0.1.1.tar.gz"
diff --git a/test/test_files.jl b/test/test_files.jl
index 7cacac3c..e12531e6 100644
--- a/test/test_files.jl
+++ b/test/test_files.jl
@@ -2,7 +2,7 @@ using Test, JLD2
 using LazyArtifacts
 # When adding test files to the JLD2Testfiles repo tag a new
 # release and adapt Artifacts.toml and the line below accordingly.
-testfiles = artifact"testfiles/JLD2TestFiles-0.1.0/artifacts"
+testfiles = artifact"testfiles/JLD2TestFiles-0.1.1/artifacts"
 
 @testset "HDF5 compat test files" begin
     # These are test files copied from the HDF5.jl test suite
@@ -133,4 +133,14 @@ end
         # Opening for overwriting should succeed
         close(jldopen(path, "w+"))
     end
+end
+
+@testset "Mutable Struct Reconstruction Issue #506" begin
+    fn = joinpath(testfiles,"struct_reconstruction.jld2")
+    data = load(fn)
+    @test data["dms"] isa JLD2.SerializedDict
+    @test JLD2.isreconstructed(data["dms"].kvvec[1].second)
+    @test data["ds"] isa JLD2.SerializedDict
+    @test JLD2.isreconstructed(data["ds"].kvvec[1].second)
+    @test JLD2.isreconstructed(data["tms"][2])
 end
\ No newline at end of file