Add a built in created time field for Tickets and the ability to filter Tickets by created time.

[sawagh]

Here are the highlights of this change:

• Adds a create_time field to Ticket proto and populates this field during Ticket creation
• Adds created_before, created_after to the Pool and filters Tickets based on create time if this field is specified.

One implementation nuance was that the current filter package directly operates on Pools. In this case (and may be similar in future), we need to convert the proto to time field - which doing for each Ticket is a lot of unnecessary repetitive work. Further, this conversion can fail - at which point, failing it at a Ticket level does not make sense (that validation / failure should be outside of the filter package).

Hence, this change abstracts the Pool from the filter package and instead introduces a Criteria structure that contains validated filtering criteria. Any validations on Pool specification can happen outside (in query service) before passing the Criteria to filter package.

One nuance is the test cases. The way the test is currently structured, the 'testcases' package is used by E2E as well as filter_tests which is great - however, with this change, the filter tests need Criteria. Hence, the subtle change is that the testcases package still uses Pool but filter_test converts that to criteria as it intends to test only the criteria whereas the e2e will continue to use more E2E Pool specification.

[Laremere]

I don't quite like how this needs to be parsed every time. It will be parsed for every ticket for every query, regardless of if it uses CreatedBefore / CreatedAfter.

We're setting this field, so it should be valid always. Hm.
Maybe we need a type for a parsed ticket too... I don't know. What are your thoughts?

If we are logging, it should be an error, because the system is definitely not behaving at any point where a field it set is invalid.

[Laremere]

Ok, possible course of action (unsure):
Create another type in this package that is created from a ticket proto. No visible fields, but it's passed to the In/Meets method. Maybe it can return the ticket that it was created from.

Then in query cache, use this object in the map instead of tickets.

If the ticket create time parsing fails, I think a reasonable thing to do is loudly log (but this will only happen when query cache reads the ticket, instead of every query), and then assume the ticket is one minute old.

[sawagh]

For your logging comment, I ve changed the log to Error.

I see your concern around Ticket parsing. I did go down the path of exploring a type for the Ticket and updating query cache with it. Now, instead of parsing time on each query, you would end up re-creating the ticket for each query. Was not great. The alternative was for that custom type to just have (time.time and Ticket), store and use the entire ticket as is and only parse the time - so query cache still uses the custom type - but internally the type just stores Ticket+time. Seems strange again.

I've added a condition now so that the time gets parsed only if using before / after - so slight improvement over the previous. The time parsing btw, is just another proto conversion - I dont think it would impact performance but again, I ve not validated that. With the condition though, atleast the impact is limited to only using the time range feature.

I would prefer to keep this change as is - and if we want to explore optimizing the time parsing, doing that as a separate change - given that is an internal only optimization.

[sawagh]

@googlebot rescan