Updated to liboscal-java 1.0.2 and addressed changes

[rgauss]

No description provided.

[rgauss]

This seems potentially brittle but we seem to have lost the more specific exception we previously used.

[david-waltermire]

I understand what you are saying here. There are three major cases where an IOException can be thrown now.

1. There is a problem reading from the underlying stream.
2. The content is malformed JSON.
3. The content is malformed OSCAL.

In all cases the content cannot be parsed. Previously 1 and 2 caused an IOException and 3 caused a BindingException. Given 2 and 3 are syntax errors, I wanted to normalize the exceptions thrown.

FWIW, there is an IJsonProblemHandler that is used as a callback from the deserializer to address OSCAL syntax issues. I believe I need to expose an option to override this on the deserializer, to allow for detection of unknown and missing content. This might be better than using the exception.

[rgauss]

FWIW, there is an IJsonProblemHandler that is used as a callback from the deserializer to address OSCAL syntax issues. I believe I need to expose an option to override this on the deserializer, to allow for detection of unknown and missing content. This might be better than using the exception.

Yeah, saw that, and yes that could be helpful.

We might also take an approach of some simple content detection first rather than jumping straight into trying to load.

[rgauss]

@david-waltermire-nist, this might be of interest. Perhaps the default behavior of Jackson (closing the stream) changed in the update?

[rgauss]

To add a bit more detail, the part of the tests that were failing essentially just creates a ByteArrayOutputStream and ends up calling ISerializerwhere writer.flush fails.

[david-waltermire]

It's weird that this is happening now. It should have happened all along. Looks like Jackson's default behavior is to auto-close the stream.

[david-waltermire]

Should I change the default behavior? I believe I should.

[rgauss]

I created a simple test in OscalBindingContextTest in liboscal-java that fails with the same error:

  @Test
  void testSerializeSspToOutputStream() throws IOException {
    SystemSecurityPlan ssp = new SystemSecurityPlan();

    IBindingContext context = IBindingContext.newInstance();
    ISerializer<SystemSecurityPlan> serializer = context.newSerializer(Format.JSON, SystemSecurityPlan.class);
    serializer.enableFeature(Feature.SERIALIZE_ROOT);
    ByteArrayOutputStream out = new ByteArrayOutputStream();
    serializer.serialize(ssp, out);
  }

That didn't seem worth creating a PR into liboscal-java and the fix into metaschema-java, but I can do that if you'd like.

[rgauss]

Yes, but the bigger concern is that the out-of-the-box metaschema serializer currently fails, regardless of what the calling code would want to do with that stream after serialization.

[david-waltermire]

Ok. I'll look into this.

[david-waltermire]

Yes. I ran your unit test. This is caused by the Jackson autoclose behavior. I am going to disable it.

[david-waltermire]

See usnistgov/metaschema-java#29.

[rgauss]

Excellent, thanks!

[david-waltermire]

@rgauss It looks like the changes here are minimal based on the liboscal-java API adjustments. Glad to see this.

[laurelmay]

@rgauss now that #94 is merged, there's a conflict here and I don't want to guess at resolving it. I think we want:

    this.deserializer = new DefaultJsonDeserializer<T>(context, classBinding);
    this.deserializer.enableFeature(Feature.DESERIALIZE_JSON_ROOT_PROPERTY);

but I'm not 100% sure.

[rgauss]

@rgauss now that #94 is merged, there's a conflict here and I don't want to guess at resolving it. I think we want:

    this.deserializer = new DefaultJsonDeserializer<T>(context, classBinding);
    this.deserializer.enableFeature(Feature.DESERIALIZE_JSON_ROOT_PROPERTY);

but I'm not 100% sure.

Yes, that was right. Rebased, should be good to go.