Skip to content

DateMath -- Null string throws exception #1766

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
RobinRieger opened this issue Jan 28, 2016 · 3 comments
Closed

DateMath -- Null string throws exception #1766

RobinRieger opened this issue Jan 28, 2016 · 3 comments
Assignees

Comments

@RobinRieger
Copy link
Contributor

I am having some problems with the DateRange filter.

The following does not work

string tempField = null;

.DateRange
    (ra => ra.Field(fi => fi.ADate)
        .LessThanOrEquals
            (!string.IsNullOrEmpty(tempField) ? DateTime.UtcNow.ToString() : (string) null))

but this does

.DateRange
    (ra => ra.Field(fi => fi.ADate)
        .LessThanOrEquals
            (!string.IsNullOrEmpty(tempField) ? DateTime.UtcNow : (DateTime?) null))

I have a feeling that the DateMath does not like null values that are 'strings'.

Keeps saying {"Value cannot be null.\r\nParameter name: input"}

From my understanding it should be able to handle this correct? I.e. if it is null then don't send the DateRange stuff to ES.

Edit
I actually get an error with other searchs as well, e.g.

.Query (que =>que.Terms<int>(td => td.Field
    (fi => fi.AField)
    .Terms (null )))

but if I pass a new int[]{} it works.....

@RobinRieger
Copy link
Contributor Author

I had some time this morning to look at this, and I think the problem is there is no 'null' handling on some of the methods.

For example in the DateMath file on line 41, i think there should be a null check e.g.

if (string.IsNullOrWhiteSpace(dateMath))
    {
        return null;
    }

because otherwise the regex check on line 43 if you pass it a null seems to throw an excpetion.

Also in the 'TermsQuery` I believe that line 68 should read:

public TermsQueryDescriptor<T, TValue> Terms(params TValue[] terms) => Assign(a => a.Terms = terms != null? terms.Cast<object>() : null);

I would have made a PR but I have two problems.

  1. I am not sure if this is actually 'wrong' or if this is by design and you should not pass null values
  2. I don't know all the places where this design pattern is used. I.e. I just stumbled across the null exception with the TermsQuery but I would think that there are other query types that use the same/similar code, I am just not confident enough yet with the code to say I can find all of them....

Can someone maybe check to see if they also get the exceptions? @russcam?

@russcam
Copy link
Contributor

russcam commented Jan 29, 2016

Thanks for digging into this @RobinRieger - intending to look at this today 👍

@russcam russcam self-assigned this Jan 29, 2016
@russcam
Copy link
Contributor

russcam commented Jan 29, 2016

DateMath defines to implicit operators for conversions from string and DateTime

[JsonConverter(typeof(DateMath.Json))]
public abstract class DateMath : IDateMath
{
    // ... rest of class
    public static implicit operator DateMath(DateTime dateTime) => DateMath.Anchored(dateTime);
    public static implicit operator DateMath(string dateMath) => DateMath.FromString(dateMath);
}

Now in the case of

string tempField = null;

client.Search<dynamic>(s => s
    .Query(q => q
        .DateRange(ra => ra
            .Field(fi => fi.ADate)
            .LessThanOrEquals(!string.IsNullOrEmpty(tempField) 
                ? DateTime.UtcNow.ToString() 
                 : (string) null)
            )
        )
    )
);

an implicit conversion from string to DateMath takes place which ends up throwing an exception when attempting to match a date math string using a regular expression on a null string. We should return null here when passed a null string instead of returning an instance of DateMath (more on this in a bit).

In the case of

string tempField = null;

client.Search<dynamic>(s => s
    .Query(q => q
        .DateRange(ra => ra
            .Field(fi => fi.ADate)
            .LessThanOrEquals(!string.IsNullOrEmpty(tempField) 
                ? DateTime.UtcNow
                 : (DateTime?) null)
            )
        )
    )
);

there is no implicit conversion from a null DateTime? to a DateMath so no implicit conversion takes place; null is therefore passed to LessThanOrEquals().

This is where Conditionless queries come into play.

  1. In the string case, because an instance of DateMath is passed to LessThanOrEquals() the query ends up being serialized, with the json converter for DateMath writing out null as a consequence of the null string passed and implicitly converted. This is an issue to address.
  2. In the DateTime? case, the query is conditionless and so is not serialized as part of the request.

Thanks for opening these issues and trying out NEST 2.x alpha - we'd like to send you some swag as a thank you; Would you be able to send me an email at russ.cam (the domain name is the same as the organisation domain website - damn the spam bots!) with your details?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants