#2013: add support for reading from / writing to a Path

[sdoeringNew]

Here, too, there seems no need for adding unit tests. 😨

That PR is the counterpart to: FasterXML/jackson-core#680

[michaelhixson]

I mentioned non-default file systems as a use case that is not supported by the java.io.File-accepting methods, but which could be supported by java.nio.file.Path-accepting methods depending on the implementation.

I think your implementation supports this use case, but do you think it's worth adding a test to verify this?

Here's a sketch of such a test. This might work as written, but I didn't try it in the context of this PR.

/**
 * Verifies that {@link ObjectMapper} can read from and write to {@link Path}s
 * whose {@link Path#getFileSystem()} is not the {@linkplain
 * FileSystems#getDefault() default file system}.
 */
public void testNonDefaultFileSystem() throws IOException {
  Map<String, Integer> value = Collections.singletonMap("a", 1);
  ObjectMapper mapper = new ObjectMapper();

  Path zipFile = Files.createTempFile("ObjectMapperTest", ".zip");
  try {
    // Write an empty zip archive to the temp file.
    try (OutputStream out = Files.newOutputStream(zipFile);
         ZipOutputStream zipped = new ZipOutputStream(out)) {
    }

    // Open the empty zip archive as a file system.
    try (FileSystem zipFs = FileSystems.newFileSystem(zipFile, (ClassLoader) null)) {
      Path jsonFile = zipFs.getPath("/test.json");
      mapper.writeValue(jsonFile, value);
      String serialized = new String(Files.readAllBytes(jsonFile), StandardCharsets.UTF_8);
      assertEquals("{\"a\":1}", serialized);
      Map<String, Integer> deserialized =
          mapper.readValue(jsonFile, new TypeReference<Map<String, Integer>>() {});
      assertEquals(value, deserialized);
    }

  } finally {
    Files.delete(zipFile);
  }
}

[sdoeringNew]

Yeah, sure. I'm always pro tests.
But have you looked at the source code? The changed classes are barely tested and missing tons of coverage. Neither the readValue(..) nor the writeValue(..) methods are unit tested.
I'm unsure if adding tests isn't breaking some sort of coding standards or guidelines.

Perhaps the maintainer can clarify.

[cowtowncoder]

Ok, just to make sure: lack of testing for File-sourced/targeted methods is likely just laziness on my part, not having originally added verification -- tests are mostly added for reported bugs/regression, and new features.
So do not take absence of tests to suggest that additions would be unwelcome.

There have been occasional efforts to extend test coverage, but that often focuses on pieces that seem more likely to break, and to be honest File-source readValue() method is relative straight-forward (and only couple of lines are distinct, rest following shared code paths that are tested).

But I think that going forward it is valuable to add tests for new code, even if there are gaps with existing methods. And I'd be +1 for adding tests for gaps too, of course, but that's more optional.

So I would like to see @michaelhixson's contribution added.

And we can start with 3.0, then think whether 2.13 would make sense or not; I have no strong opinion there.

With that, just one more thing @sdoeringNew : unless I have asked for and received CLA (from https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf), that'd be needed.
The common way is to just print the doc, fill & sign, scan/photo, email to info at fasterxml dot com.
Only need to be done once, before the first merge, good for all Jackson contributions afterward.

Looking forward to getting this merged!

[sdoeringNew]

Besides @michaelhixson test case I've added some more Unit tests.

The contribution has been signed and send.

[cowtowncoder]

Typically should verify exception message; I can add that.

[cowtowncoder]

Thank you @sdoeringNew -- as with jackson-core, addition of test much appreciated!

[cowtowncoder]

Will change p to path just because p traditionally refers to JsonParser (or whatever it'll get renamed as)