Skip to content
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

External Tasks are Sorted By Creation Date - Backend & Java Client #3948

Closed
5 tasks done
Tracked by #3896
psavidis opened this issue Nov 14, 2023 · 5 comments
Closed
5 tasks done
Tracked by #3896

External Tasks are Sorted By Creation Date - Backend & Java Client #3948

psavidis opened this issue Nov 14, 2023 · 5 comments
Assignees
Labels
type:subtask Issues that are subtasks of another issue. Must always be part of the breakdown of the parent issue. version:7.21.0-alpha4 version:7.21.0

Comments

@psavidis
Copy link
Contributor

psavidis commented Nov 14, 2023

Acceptance Criteria (Required on creation)

  • Add the capability of sorting by createTime to the Query API

  • Adjust Persistence Layer to Sort by Create Time

    • Make sure the Queries that retrieves External Tasks & apply the Sorting Logic consider the new parameter for ASC & DESC orders. We don’t implement logic that brings null values into a specific order. We let the database vendor decide how it is handled.
    • Adjust the queries: selectExternalTasksForTopics & selectExternalTaskByQueryCriteria for Fetch & Lock API & Query API Respectively.
    • MyBatis Adjustments: ExternalTaskEntity, Interface, Insert Query, ResultMap & columnSelection (see ExternalTask.xml)
    • Add Unit Test that guarantees the new Sorting Behaviour for different Db profiles.
  • Add Sort by Create Time to Fetch and Lock API

    • Add the parameter to the Fetch and Lock API of External Tasks

    • Currently usePriority implementation defines the order statically to DESC. Change this to so that ordering can work both for ASC and DESC configuration for prioritising create-time in LIFO and FIFO orders.

    • Implement Multi-Level Sorting By Considering Fields in the following order:

      • First priority is taken into account
      • In the equality scenario, create time will be used next.
      • The above order should be preserved for the Fetch and Lock API
      • For the External Task Query API, the multi-level sorting is handled consistently with all other Query APIs via the #orderBy… fluent builder pattern.
  • Add Create Time to Open API

  • Add the Sort by Create Time Capability to the Java Client

    • Expose the new parameter and fetch and lock Sorting options to the Java client
    • When useCreateTime is set to true and no Sorting Direction is specified (null value), DESC will be used as a sensible default
    • When useCreateTime and usePriority are both enabled, the Sorting should take place by:
    • First by applying priority DESC
    • On priority equality, the selected createTime direction should be used

Hints

Links

Breakdown

Pull Requests

Preview Give feedback
  1. ci:all-db
    psavidis
  2. ci:all-db
    psavidis
  3. ci:all-as
    psavidis
  4. psavidis
  5. ci:all-as ci:spring-boot
    psavidis
@psavidis psavidis added the type:subtask Issues that are subtasks of another issue. Must always be part of the breakdown of the parent issue. label Nov 14, 2023
@psavidis psavidis self-assigned this Nov 14, 2023
@psavidis psavidis changed the title External Tasks are Sorted By Creation Date - Backend External Tasks are Sorted By Creation Date - Backend & Client Nov 14, 2023
@psavidis psavidis changed the title External Tasks are Sorted By Creation Date - Backend & Client External Tasks are Sorted By Creation Date - Backend & Java Client Nov 14, 2023
@psavidis psavidis assigned mboskamp and unassigned psavidis Nov 20, 2023
@mboskamp
Copy link
Member

@psavidis, this ticket is in progress but assigned to me. What is the status?

@psavidis
Copy link
Contributor Author

@psavidis, this ticket is in progress but assigned to me. What is the status?

I've corrected the status to Review. The changes are applied for all the Code Review points. We can review them further.

@mboskamp
Copy link
Member

I have reviewed the engine and engine-rest changes. Let's discuss those before I move on to dependent PRs.

@mboskamp mboskamp assigned psavidis and unassigned mboskamp Nov 23, 2023
@psavidis
Copy link
Contributor Author

psavidis commented Nov 30, 2023

The Code review of the backend is done and can proceed. Also the client points have been addressed too.

The spring boot client integration can be part of the client since the changes are not numerous.

@psavidis
Copy link
Contributor Author

psavidis commented Dec 4, 2023

Update for the Review :

For the Spring-Boot integration of the Client, the following agreements have been made with the tech lead:

  • When useCreateTime & orderByCreateTime have been configured via Spring Boot configuration, it makes sense to fail with an exception (SpringExternalTaskClientException)
  • When the user configures a value that is neither asc or desc, the client will throw an SpringExternalTaskClientException.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:subtask Issues that are subtasks of another issue. Must always be part of the breakdown of the parent issue. version:7.21.0-alpha4 version:7.21.0
Projects
None yet
Development

No branches or pull requests

4 participants