dblevins commented on PR #84: URL: https://github.com/apache/johnzon/pull/84#issuecomment-1107906383
You're right -- I missed the code for JohnzonBuilder and the wildcard builder. I went ahead and added it along with a test case to verify the `johnzon.snippetMaxLength` parameter works. The supplied `JsonGeneratorFactory` is getting used. I suspect the feedback is for the second constructor that creates a JsonGeneratorFactory when one isn't available. I updated it to construct `JsonGeneratorFactoryImpl` directly and documented it is not the preferred constructor. Hopefully that addresses the feedback. The `close` call was there due to buffering that would lead to an empty string. Those calls are documented as idempotent and safe to call more than once, so my thinking was "close is better than flush." I dug into that a bit more and found the buffering was causing more significant issues and hiding some bugs. The short version: the 64k buffer in JsonGenerator was causing the whole document to be serialized despite our `isComplete()` calls. The SnippetOutputStream would never see any bytes until the very end, so ultimately this code was no better than the `toString()` version I originally submitted. I've significantly amped up the testing, reworked the code and all is looking good -- famous last words. Give it a good look and let me know if anything looks off. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@johnzon.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
