Repository navigation
Fix NDJSON and CSV export handling of existing file contents - #531
Open
officialasishkumar wants to merge 1 commit into
Open
officialasishkumar wants to merge 1 commit into
officialasishkumar wants to merge 1 commit into
Conversation
Exporting in append mode did not account for the state of the file already on disk: - NDJSON::export() wrote the first new record directly after the last byte of the file. If the existing file did not end with a newline, the first new record was glued onto the previous line, producing invalid NDJSON that the extractor could no longer read. It now inserts a line separator before the first record in that case, mirroring the fix applied to the CSV extractor in 35864a4. - CSV::export() decided whether a leading separator was needed without looking at the overwrite flag. With header disabled and overwrite enabled, the file was truncated and then a stray blank line was written before the first row. The separator is now only considered when appending. - CSV::export() only wrote the header when the file did not exist. An existing but empty file (e.g. one created by tempnam() or touch) got no header, so reading it back with the header enabled consumed the first data row as the header and silently dropped it. Empty files are now treated the same as missing files. Both exporters clear the stat cache for the target path before inspecting it so that file size checks reflect writes made by earlier exports in the same process. Fixes RubixML#529
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #529
Exporting in append mode didn't take the existing file on disk into account. This caused three problems:
JSON Error on line N: Syntax error).NDJSON::export()now writes a line separator first in that case. This is the same fix 35864a4 made for CSV.$overwrite. Withheader: false, overwriting a file that had no trailing newline truncated it and then wrote a stray blank line before the first row. The check now only runs when appending.tempnam()ortouch, got no header. Reading it back withheader: truethen used the first data row as the header and silently lost it. Empty files are now treated like missing files.Both exporters now call
clearstatcache()for the target path before checking it. Otherwisefilesize()could return a stale value from before an earlier export in the same process.Tests
CSVTest::exportOverwriteNoTrailingNewline: no leading blank line when overwriting.CSVTest::exportAppendToEmptyFileWritesHeader: the header is written to an empty file and is not repeated on the next append.NDJSONTest::exportAppendNoTrailingNewline: a separator is added when appending to a file without a trailing newline, and none is added when the file already ends with one.NDJSONTest::exportOverwriteNoTrailingNewline: no separator is written when overwriting.The first three fail on
master. The full suite, PHPStan and the coding style check pass.