Added support for filtering by unused parameter for HeapMemoryTaskStorage

[Guadrado]

1. added support for unused DateTime start parameter in getRecentlyFinishedTaskInfoSince method:

• to fix it DateTime modifiedDate field added to TaskStuff class
• withStatus method call creates new TaskStuff with new status and current modifiedDate
• TaskStuff list stream return elements, which status is complete and modified Datetime is after start DateTime

2. added filtering by status complete to TaskStuff list stream in getNRecentlyFinishedTaskInfo method.

[jihoonson]

Hi @Guadrado thank you for the contribution!

Even though the javadoc of getRecentlyFinishedTaskInfoSince is saying No particular standard of "recent" is guaranteed, I think it makes sense to use the current behavior of MetadataTaskStorage since it's being used as the standard metadata storage in productions and we want to make all implementations of TaskStorage consistent.

In MetadataTaskStorage, getRecentlyFinishedTaskInfoSince returns the finished tasks which are created after the given time. HeapMemoryTaskStorage can also do this by comparing TaskStuff.createdDate with the given time. What do you think?

[Guadrado]

Hi @jihoonson! Thank you for comment!

I completely agree with you, that all implementations of TaskStorage must be consistent.
But if we imagine a developer, who knows nothing about the internal implementation of OverlordResource API, he call public API method OverlordResource.getTasks (which internally call storage.getRecentlyFinishedTaskInfo) with state "complete" and interval parameters,
he would expect, that method would return the list of tasks, which date of completion , not createdDate, belongs to the interval.

Now i see two ways:

1. If this behavior of API method is legal and tasks must be filtered by TaskStuff.createdDate,
   I propose to change names like that:
   in public API method OverlordResource.getTasks parameter interval -> createdTimeInterval
TaskStorageQueryAdapter.getRecentlyCompletedTaskInfo-> TaskStorageQueryAdapter.getCompletedTaskInfoByCreatedTimeDuration
TaskStorage.getRecentlyFinishedTaskInfo -> TaskStorage.getFinishedTaskInfoByCreatedTimeDuration
   This will avoid misunderstanding.
   In this case HeapMemoryTaskStorage can return the finished tasks by comparing TaskStuff.createdDate with the given time.

2. If there is a need for an API method, that will return a list of tasks filtered by the date, when the required state was established, for example, filtered by the date of completion for the status "complete", I suggest, in addition to the first way, to implement the following:
   add parameters statusedDateInterval to public API method OverlordResource.getTasks,
   add method TaskStorageQueryAdapter.getCompletedTaskInfoByFinishedTimeDuration,
   add method TaskStorage.getFinishedTaskInfoByFinishedTimeDuration,
   and make the appropriate changes to MetadataTaskStorage and HeapMemoryTaskStorage.

[jihoonson]

@Guadrado thanks for the detailed comment! I think 1. would be better because 2. would cause the incompatibility issue in both metadata storage and overlord API because we don't store other times like complete time in metadata store and apps based on the overlord API is assuming the result is filtered by the creation time.

[Guadrado]

@jihoonson OK. Thank you for response! Could you tell me how I can add appropriate changes of 1.? Should I close this pull request and open new one?

[jihoonson]

@Guadrado I think it's fine to update this PR. Thanks!

[jihoonson]

@Guadrado thanks for the update. I left a few comments about naming.

[jihoonson]

@Guadrado sorry for delayed review. The latest change looks good to me. Would you please fix the conflict?

[Guadrado]

@Guadrado sorry for delayed review. The latest change looks good to me. Would you please fix the conflict?

@jihoonson, yes, of course.

[jihoonson]

@Guadrado thanks for the quick update! Unfortunately, the build failed because of the recent change which prohibits String.replace() (#6607). You might use StringUtils.replace() instead. Would you please fix it?

[Guadrado]

@Guadrado thanks for the quick update! Unfortunately, the build failed because of the recent change which prohibits String.replace() (#6607). You might use StringUtils.replace() instead. Would you please fix it?
@jihoonson yes, sure.

[jihoonson]

@Guadrado thank you! I'm merging this PR shortly.

[jihoonson]

I also changed the title to be more clear.

[Guadrado]

@Guadrado thank you! I'm merging this PR shortly.

@jihoonson thank you for your help!
I'm glad, that I can take a part in this project.