codecutout / StringToExpression

Convert strings into .NET expressions
MIT License
95 stars 21 forks source link

oData Dates #10

Open JohnGalt1717 opened 4 years ago

JohnGalt1717 commented 4 years ago

Right now filters like this: CreatedOn ge 2019-10-01T00:00:00.000Z

which is what the odata 4 standard says, doesn't seem to work.

Unexpected token 'T' found at StringToExpression.Tokenizer.Tokenizer.Tokenize(String text)

JohnGalt1717 commented 4 years ago

I was sort of able to address this by inheriting from the odata language definition and removing the default Datetime items and inserting these (appending didn't work because the string regex matches first that way)

        protected override IEnumerable<GrammerDefinition> TypeDefinitions()
        {
            var defs = new List<GrammerDefinition>(base.TypeDefinitions());

            defs.RemoveAll(d => d.Name == "DATETIMEOFFSET" || d.Name == "DATETIME");

            defs.InsertRange(0, new[] {
                new OperandDefinition(
                    name: "DATETIMEOFFSET",
                    regex: @"\d{4}-\d{1,2}-\d{1,2}T\d{1,2}:\d{1,2}:\d{1,2}\.\d{1,3}Z",
                    expressionBuilder: x => Expression.Constant(DateTimeOffset.Parse(x))
                ),
                new OperandDefinition(
                    name: "DATETIME",
                    regex: @"\d{4}-\d{1,2}-\d{1,2}T\d{1,2}:\d{1,2}:\d{1,2}\.\d{1,3}Z",
                    expressionBuilder: x => Expression.Constant(DateTime.Parse(x))
                )
            });

            return defs;
        }

However, this gives me a error in .NET Core 3.1 EF that it can't interpret the Query, so I don't know what I'm doing wrong.

Wondering if it's that DATETIME would never be called there. Need to be able to make this more contextual so that it applies the right one for the right type of the field itself, not just the regex.

alex-davies commented 4 years ago

The parser was originally made for OData2, which is why it does not defaultly parse the dates.

What you are doing seems correct, a few comments

As to why it is failing on 3.1 EF I am not sure, can you output the expression tree you are passing to EF and see if there is anything odd generated in there?

Id have no issues having date literal parsing in the OData language provider and would happily accept PRs. Would need to get a more generic regex than can handle most ISO8601 dates as well as handling the implicit casting of datetimes (Easist way would be to just add DateTime and DateTimeOffset to https://github.com/codecutout/StringToExpression/blob/master/src/StringToExpression/Util/ExpressionConversions.cs#L15-L29). But understand if you are focused on your particular use case, the parser should be flexible enough that you can implement this without making core code commits

JohnGalt1717 commented 4 years ago

Ok, so we use only DateTimeOffset, and that code you linked to doesn't appear to have DateTime in it either.

I was able to get DateTimeOffSet working by just replacing the entire class myself in code. If I just overrode it, it didn't work. I think this might have something to do with EF Core 3.1 and versions of .net standard.

I have never done a PR before. I'm happy to do one if you can point me to how you want it done and I'll add the new OData4 code. And if you can be a little more specific on your code link I can put that in too.

bookwood1977 commented 3 years ago

Check the test project.

There are a lot og unit test. that is great. But somehow the tests are incomplete.

The tests check datetime doesn't contain any timezone information. Check datetimeoffset?` not a single test for.

I have issues with "possitive" timezones.

GMT oder Negative - runs well. but datetime oder datetimeoffset string containing '+' are not working. it seems the '+' is removed while processing.

Then... no valid datetime...