Skip to content

Refactor AddTaskHandler and AddTaskToTaskwarrior to improve maintainability and performance #371

@its-me-abhishek

Description

@its-me-abhishek

Description
The current task creation pipeline (from AddTaskHandler to AddTaskToTaskwarrior) suffers from high memory overhead, unnecessary CLI/DB calls, and a brittle function signature. This refactor targets performance bottlenecks and improves maintainability by consolidating data structures.

Changes Required:

  1. Optimize Request Parsing

    • Replace io.ReadAll and json.Unmarshal with json.NewDecoder.
    • Reason: json.NewDecoder streams the request body directly, which is more memory-efficient than reading the entire body into a byte slice before parsing.
  2. Check dependencies only when they're not null

existingTasks, err := tw.FetchTasksFromTaskwarrior(email, encryptionSecret, origin, uuid)
		if err != nil {
			if err := utils.ValidateDependencies(depends, ""); err != nil {
				http.Error(w, fmt.Sprintf("Invalid dependencies: %v", err), http.StatusBadRequest)
				return
			}
		} else {
			taskDeps := make([]utils.TaskDependency, len(existingTasks))
			for i, task := range existingTasks {
				taskDeps[i] = utils.TaskDependency{
					UUID:    task.UUID,
					Depends: task.Depends,
					Status:  task.Status,
				}
			}

			if err := utils.ValidateCircularDependencies(depends, "", taskDeps); err != nil {
				http.Error(w, fmt.Sprintf("Invalid dependencies: %v", err), http.StatusBadRequest)
				return
			}
		}

This can prevent the unnecessary fetching of tasks. Which will then reduce the overall response time.

// Only fetch and validate if dependencies exist
if len(requestBody.Depends) > 0 {
    existingTasks, err := tw.FetchTasksFromTaskwarrior(requestBody.Email, ...)
    // ... validation logic
}
  1. Parameter Bloat: The function signature currently takes 15 arguments.
    • Fix: Change the signature to accept a models.AddTaskRequestBody. Can use this inside the AddTasktoTW as well as the handler.
    • Exception: dueDateStr should be passed as a second argument since it requires pre-processing/formatting before being sent to the TW CLI.
    • Quite similar to func AddTaskToTaskwarrior(req models.AddTaskRequestBody, dueDateStr string)
    • An alternative could be to compute this dueDateStr directly inside the AddTaskToTaskwarrior function. Making the handler cleaner

Acceptance Criteria

  • Handler uses json.NewDecoder.
  • Task fetching is skipped if Depends is empty.
  • AddTaskToTaskwarrior signature is consolidated.
  • Log output is detailed enough to reconstruct a failure without a debugger.
  • 100% test coverage on the new logic paths in AddTaskHandler.
  • The code is manually reviewed and relevant comments are present inside the code-review.

Metadata

Metadata

Assignees

No one assigned

    Labels

    backendSomething isn't working on the backendenhancementNew feature or requestgood first issueGood for newcomers

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions