Fix #151 - support Timestamp precision in text format

[CoderNate]

Support Precision values other than Second in the text format.

I also noticed that timestamps with DateTimeKind.Utc seemed to always have +00:00 on the end instead of Z because AsDateTimeOffset doesn't know if a timestamp is supposed to be UTC.

This isn't really a backwards compatible change so not sure if this would require a major version bump or something...

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[popematt]

Thanks for this contribution. The code looks pretty good to me, but I have one question just to make sure we're all on the same page here. The PR title says "in text format" (which could refer to Ion text), but the code changes are for writing JSON. You are trying to fix the text encoding when downgrading to JSON rather than when writing the Ion text encoding, right?

[CoderNate]

The PR title says "in text format" (which could refer to Ion text), but the code changes are for writing JSON.

Ah I wasn't thinking very carefully about the difference between ion text vs json. This PR affects both because the text writer just calls ToString regardless of if it's Amazon.IonDotnet.Builders.IonTextOptions.Json or Amazon.IonDotnet.Builders.IonTextOptions.Default:

ion-dotnet/Amazon.IonDotnet/Internals/Text/IonTextWriter.cs

Line 241 in 74c1746

public override void WriteTimestamp(Timestamp value)

I think this PR is mostly what you want and I think things like Newtonsoft.Json would still be able to automatically detect strings that look like dates or date/times and turn it into a native date or date/time value in the case of Day or Second precision. However, before this PR you would have been able to write a timestamp with something like Year precision and it would have created something like "2010-01-01T00:00:00.0000000-00:00" which loses precision info but would still be auto-detected by Newtonsoft as a DateTime (assuming the default settings).

Regardless of how .NET packages behave I guess the best is to do what the other Ion implementations do. Do you happen to know what the other ones do when writing a Timestamp with Year or Month or Minute precision when in JSON mode? Maybe it's configurable how it should behave?

[popematt]

This PR affects both because the text writer just calls ToString regardless of if it's Amazon.IonDotnet.Builders.IonTextOptions.Json or Amazon.IonDotnet.Builders.IonTextOptions.Default

before this PR you would have been able to write a timestamp with something like Year precision and it would have created something like "2010-01-01T00:00:00.0000000-00:00"

Oh wow, this bug is worse than I thought. That's definitely wrong, and should not be configurable. An Ion Timestamp should always be written with the correct precision in Ion format.

I'm pretty confident that your code change is correct, but we're going to have to dive into this more just to see what else might be going on.

[CoderNate]

An Ion Timestamp should always be written with the correct precision in Ion format.

Yeah 100%, I was wondering only for JSON mode if maybe there were different ways to handle it 👍

we're going to have to dive into this more just to see what else might be going on

Sounds great, thanks!

[popematt]

Well, I figured out why this bug wasn't caught before. The unit tests for the project are not set up correctly. Running dotnet test does not actually run any tests.

I was able to make some changes to get the tests running, but a lot of the tests are failing. However, all of the tests related to your change are passing, so I'm going to approve and merge your PR.

[popematt]

@CoderNate We don't have a lot of information about how people are using ion-dotnet. Do you mind telling us a little bit about what you're doing with it? Nothing too detailed—we don't want you to share any IP. Are you using this for commercial or personal projects? Are you using Ion to interact with AWS QLDB or do you use Ion for something else?

[CoderNate]

Thanks for accepting my PR 😃

My company uses ion. I hadn't heard of it previous to that but I'm definitely liking it more than msgpack! I don't think anyone at the company is using it with any AWS services. I made an internal higher-level wrapper library (https://github.com/amzn/ion-object-mapper-dotnet didn't seem like a great fit when I last looked at it).