JuliaIO / JSON.jl

JSON parsing and printing
Other
313 stars 101 forks source link

Introduce a recursive cycle check when writing #345

Closed quinnj closed 1 year ago

quinnj commented 2 years ago

I noticed that writing can blow up when there are recusrive objects that reference each other. (For example, in HTTP.jl, Response and Request reference each other) This PR proposes a simple API for the CompactContext and PrettyContext where objectid of objects will be tracked recursively when writing and it's configurable what should be written out when a recursive cycle is detected.

Custom contexts can "hook in" to this behavior by subtyping RecursiveCheckContext and including the required fields (see docs for new context). Otherwise, there shouldn't be any functional change to APIs in any way.

cc: @vilterp (oops, wrong ping the 1st time)

codecov[bot] commented 2 years ago

Codecov Report

Merging #345 (df46bcd) into master (de16550) will decrease coverage by 0.19%. The diff coverage is 96.29%.

@@            Coverage Diff             @@
##           master     #345      +/-   ##
==========================================
- Coverage   98.63%   98.44%   -0.20%     
==========================================
  Files           5        5              
  Lines         441      451      +10     
==========================================
+ Hits          435      444       +9     
- Misses          6        7       +1     
Impacted Files Coverage Δ
src/Writer.jl 97.60% <96.29%> (-0.67%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update de16550...df46bcd. Read the comment docs.

quinnj commented 2 years ago

@KristofferC or @fredrikekre, could I bug you to review here?