add new job activity endpoint

[ikrispin]

No description provided.

[github-actions]

Here's a summary of the review, focusing on pressing issues and providing concise suggestions:

---

Review Summary

The changes introduce a new DTO and an endpoint for 24-hour job activity. While the new DTO and repository method are well-defined, the StatisticService.getJobActivity24h() method is highly inefficient, performing too many database queries which will cause significant performance issues. Additionally, the JobResource's exception handling needs to be more robust and secure.

---

Pressing Issues & Suggestions

1. Critical Performance Bottleneck in StatisticService.getJobActivity24h()

• Issue: The method makes 96 individual database queries for a single API call, leading to severe performance problems under load. This is an inefficient algorithm for aggregation.
• Suggestion: Refactor to use a single, more complex database query with GROUP BY (e.g., GROUP BY FUNCTION('DATE_TRUNC', 'hour', j.createdAt), j.status). Process the aggregated results in memory. This drastically reduces database roundtrips.

2. Exception Handling in JobResource.getJobActivity24h()

• Issue: Broad catch (Exception e) and returning e.getMessage() directly. This can expose sensitive internal details (security risk) and lacks specific error handling.
• Suggestion:
  • Catch more specific exceptions expected from the service layer.
  • For INTERNAL_SERVER_ERROR, return a generic error message (e.g., "An unexpected error occurred.") to the client.
  • Log the full exception details (including stack trace) internally on the server for debugging.

---

Other Suggestions

• StatisticService.getJobActivity24h() - Timestamp Formatting:

  • Suggestion: Use DateTimeFormatter.ISO_LOCAL_DATE_TIME.format(hourStart) for consistent and explicit timestamp formatting in the DTO.

• JobActivityDataPointDto - Timestamp Type:

  • Suggestion: Consider using java.time.Instant or OffsetDateTime instead of String for the timestamp field. This provides type safety and better date-time handling.

Review Summary:
The changes introduce a new DTO for job activity, a repository method to count jobs by status within a time window, and a service method to retrieve 24-hour job activity. The new DTO and repository method are well-designed and clear. However, the StatisticService.getJobActivity24h() method, while functionally correct, is highly inefficient and will likely cause performance issues.

---

File: src/main/java/com/redhat/sast/api/repository/JobRepository.java
No major issues.
The countByStatusInTimeWindow method is clear, concise, and uses Panache's capabilities effectively. The time window definition (>= startTime AND < endTime) is a good practice for consistent interval handling.

---

File: src/main/java/com/redhat/sast/api/service/StatisticService.java
Issue: Performance inefficiency due to excessive database queries.
Comment: The getJobActivity24h() method makes 96 database queries (24 hours * 4 statuses) for a single API call. This will become a significant performance bottleneck under load and is an inefficient algorithm for this type of aggregation.

Suggestion: Refactor this method to perform fewer, more complex database queries that can aggregate the data more efficiently.

1. Option 1 (Preferred): Single Database Query with Grouping:
   Construct a single database query (or a few) that fetches job counts grouped by status and HOUR (or DATE_TRUNC('hour', createdAt)) for the last 24 hours. This would typically involve using GROUP BY and potentially CASE statements or PIVOT functionality depending on the database.
   Example (conceptual HQL/JPQL for grouping by hour and status):

   SELECT FUNCTION('DATE_TRUNC', 'hour', j.createdAt), j.status, COUNT(j)
   FROM Job j
   WHERE j.createdAt >= :twentyFourHoursAgo AND j.createdAt < :now
   GROUP BY FUNCTION('DATE_TRUNC', 'hour', j.createdAt), j.status
   ORDER BY FUNCTION('DATE_TRUNC', 'hour', j.createdAt) ASC

   Then, process this much smaller result set in memory to construct the List<JobActivityDataPointDto>.

2. Option 2 (Less Preferred but better than current): Batch Queries:
   If a single complex query is difficult with Panache, consider fetching all jobs within the 24-hour window first, then processing them in memory. This still fetches more data than needed but avoids 96 individual COUNT queries.

Suggestion: Consistency for Timestamp Formatting.
Comment: While hourStart.toString() works for LocalDateTime, it doesn't explicitly guarantee a standard format (like ISO 8601) across different environments or Java versions.
Suggestion: Use DateTimeFormatter.ISO_LOCAL_DATE_TIME.format(hourStart) for explicit and consistent formatting, which is generally more robust for API responses.

---

File: src/main/java/com/redhat/sast/api/v1/dto/response/JobActivityDataPointDto.java
No major issues.
This new DTO is straightforward and clear for representing the job activity data points.
Here's a review of the provided changes:

---

Review Summary

The changes introduce a new DTO for job activity data and a new endpoint to retrieve 24-hour job activity. The DTO is well-defined. The new endpoint correctly delegates logic to a service. However, the exception handling in the new resource method needs improvement for robustness and security.

---

File: src/main/java/com/redhat/sast/api/v1/dto/response/JobActivityDataPointDto.java

Comments:

• Minor Suggestion (Robustness): Consider using a java.time.Instant or OffsetDateTime for the timestamp field instead of String. This provides type safety, avoids potential parsing issues, and ensures consistent date-time representation. If String is kept, ensure the format (e.g., ISO 8601) is clearly documented and consistently used.

---

File: src/main/java/com/redhat/sast/api/v1/resource/JobResource.java

Comments:

• Improvement (Exception Handling - Specificity): The catch (Exception e) block is too broad. Narrow it down to more specific exceptions that statisticService.getJobActivity24h() is expected to throw (e.g., custom ServiceException, IOException if data access is involved).
• Security Concern (Information Leakage): Returning e.getMessage() directly in the INTERNAL_SERVER_ERROR response can expose internal application details or stack traces, which is a security risk. For production APIs, return a generic error message (e.g., "An unexpected error occurred while retrieving job activity.") and log the full exception details (including stack trace) on the server side for debugging.
• Clarity/Consistency (Path Naming): The path /activity/24h is clear. If future requirements might include different time ranges (e.g., 7d, 1w), consider making the duration a path or query parameter (e.g., /activity?duration=24h) for better reusability, though for a fixed 24h endpoint, the current approach is acceptable.

[github-actions]

Okay, I've reviewed the changes. Here's a summary highlighting pressing issues and providing code suggestions.

Review Summary

The pull request introduces new features for job activity reporting and refines the logic for false positive file usage. While some changes are positive (e.g., default sorting, UrlValidationService introduction, Jackson configuration), there are critical performance and security issues that need immediate attention.

Key Pressing Issues:

1. Severe Performance Bottleneck (StatisticService): The getJobActivity24h method performs 96 database queries for a single API call, which is highly inefficient.
2. Critical Security/Robustness Flaw (JobResource): Generic exception catching and direct exposure of internal exception messages poses a security risk (information disclosure) and poor error handling.
3. Logical Flaw (JobService): shouldUseFalsePositiveFile can incorrectly return true when no false positive URL exists, leading to failed attempts to access a non-existent file.
4. API Design - Timestamp Ambiguity: String timestamps in DTOs and LocalDateTime.toString() in the service can lead to timezone issues for API consumers.

---

File: src/main/java/com/redhat/sast/api/repository/JobRepository.java

• Clarity: Good. Default descending sort on createdAt is a sensible default.
• Simplicity: The new countByStatusInTimeWindow is clear and useful.
• Duplication: Minor repetition of Sort.by("createdAt").descending() is acceptable for now.

---

File: src/main/java/com/redhat/sast/api/service/JobService.java

• Pressing Issue (Logical Flaw): The shouldUseFalsePositiveFile logic can return true even if job.getKnownFalsePositivesUrl() is null. If there's no URL, there's no file to use.
  • Code Suggestion: Correct the logic to explicitly return false if job.getKnownFalsePositivesUrl() is null, regardless of other conditions, when deciding to "use" the file.

  private boolean shouldUseFalsePositiveFile(Job job, JobCreationDto jobCreationDto) {
      Boolean useFromDto = jobCreationDto.getUseKnownFalsePositiveFile();
      boolean explicitDecision = (useFromDto != null) ? useFromDto : true; // Default to true if not specified
  
      // If the DTO explicitly said NOT to use it, then don't.
      if (!explicitDecision) {
          return false;
      }
  
      // If a URL is required but not provided, we cannot use the file.
      if (job.getKnownFalsePositivesUrl() == null) {
          LOGGER.info("No known false positives URL provided for package '{}'. Setting useKnownFalsePositiveFile to false.", job.getPackageNvr());
          return false;
      }
  
      // Now check accessibility if a URL exists and we're supposed to use it.
      if (urlValidationService.isUrlAccessible(job.getKnownFalsePositivesUrl())) {
          return true;
      }
  
      LOGGER.info("Known false positives file not found for package '{}' at URL: {}. Setting useKnownFalsePositiveFile to false.", job.getPackageNvr(), job.getKnownFalsePositivesUrl());
      return false;
  }

• Pressing Issue (Security - Meta): The introduction of UrlValidationService is a strong positive for security. However, its implementation is critical for preventing SSRF attacks.
  • Suggestion: Ensure urlValidationService.isUrlAccessible() uses an allow-list approach, disallows internal IPs/hostnames, prevents redirects to untrusted destinations, and handles all URL encoding variations robustly.

---

File: src/main/java/com/redhat/sast/api/service/StatisticService.java

• Pressing Issue (Inefficient Algorithm - CRITICAL): getJobActivity24h performs 24 hours * 4 statuses = 96 database queries per API call. This will severely impact performance.
  • Code Suggestion: Refactor this to use a single, aggregated database query that groups by hour and status. Most databases can efficiently handle this (e.g., using DATE_TRUNC or similar functions). Process the single result set in Java.

  -- Conceptual SQL for aggregation (adapt for Panache/JPA specific syntax)
  SELECT
      DATE_TRUNC('hour', createdAt) AS hour_bucket,
      status,
      COUNT(*) AS job_count
  FROM job
  WHERE createdAt >= NOW() - INTERVAL '24 hours'
  GROUP BY 1, 2
  ORDER BY 1, 2;

• Pressing Issue (API Design): The timestamp field is set using hourStart.toString(), which is LocalDateTime.toString() and lacks timezone information, leading to ambiguity.
  • Code Suggestion: Use java.time.Instant or java.time.OffsetDateTime for the timestamp to ensure timezone-aware and unambiguous time representation.

---

File: src/main/java/com/redhat/sast/api/v1/dto/response/JobActivityDataPointDto.java

• Pressing Issue (API Design): The timestamp field is a String.
  • Code Suggestion: Change the timestamp field type from String to java.time.Instant or java.time.OffsetDateTime. This improves type safety and aligns perfectly with the Jackson configuration in application.properties.

---

File: src/main/java/com/redhat/sast/api/v1/resource/JobResource.java

• Pressing Issue (Exception Handling - CRITICAL): Catching a generic Exception and returning e.getMessage() directly is a major security vulnerability (information disclosure) and poor practice.
  • Code Suggestion:
    1. Catch Specific Exceptions: Avoid Exception. Catch more specific exceptions that statisticService.getJobActivity24h() might throw (e.g., custom ServiceException).
    2. Deliberate Error Response: For unexpected INTERNAL_SERVER_ERROR (500), return a generic, user-friendly message (e.g., "An unexpected error occurred. Please try again later.") to the client.
    3. Log Full Details: Log the actual exception details (including stack trace) securely on the server side for debugging.
    4. Global Exception Handling: Consider implementing a global exception mapper (ExceptionMapper in JAX-RS/Quarkus) for consistent, secure, and user-friendly error responses across the entire API.

---

File: src/main/resources/application.properties

• Clarity/Best Practice: The addition of quarkus.jackson.write-dates-as-timestamps=false and quarkus.jackson.timezone=UTC is excellent. This standardizes date serialization to ISO-8601 in UTC and is a recommended best practice for APIs.

Okay, I've reviewed the provided diffs. Here's a summary and per-file comments:

Review Summary

The changes are generally positive, improving data retrieval with default sorting and adding a utility method in the repository. The service layer change indicates the introduction of a UrlValidationService, which is a good practice from a security perspective, though its implementation and usage are not visible in this diff. The modification to shouldUseFalsePositiveFile suggests a more robust decision-making process.

---

File: src/main/java/com/redhat/sast/api/repository/JobRepository.java

Clarity & Simplicity:

• Adding Sort.by("createdAt").descending() to the paginated job queries is a good default. Users typically expect to see the most recent items first.
• The new countByStatusInTimeWindow method is clear, concise, and provides useful functionality, likely for metrics or reporting.

Duplication & Complexity:

• The Sort sort = Sort.by("createdAt").descending(); line is repeated four times within findJobsWithPagination. While minor, it could be extracted into a common find helper method if the logic becomes more complex or if dynamic sorting were introduced. For now, it's acceptable.

Inefficient Algorithm:

• No inefficient algorithms are introduced. Panache handles the underlying query optimization.

Suggestions:

• Consider making the Sort object an optional parameter in findJobsWithPagination if there's a future need for dynamic sorting based on user input (e.g., sort by package name, status, or different directions). If not, the current default is perfectly fine.

---

File: src/main/java/com/redhat/sast/api/service/JobService.java

Clarity & Simplicity:

• The addition of private final UrlValidationService urlValidationService; indicates an important step towards robust input validation, especially if URLs are user-supplied.
• Changing shouldUseFalsePositiveFile to accept the Job object as well (shouldUseFalsePositiveFile(job, jobCreationDto)) suggests that the decision now considers the final persisted job's properties, which often leads to more accurate logic.

Security:

• UrlValidationService: The introduction of UrlValidationService is a strong positive for security. It implies that user-provided URLs will be validated, which is crucial for preventing vulnerabilities like Server-Side Request Forgery (SSRF) and ensuring data integrity.
  • Suggestion: While the implementation isn't in this diff, I highly recommend a thorough review of the UrlValidationService itself to ensure it uses an allow-list approach, effectively handles various URL schemes, protects against bypasses (e.g., IP address variations, URL encoding tricks), and prevents redirects to untrusted destinations. Its effectiveness is paramount.

Duplication & Complexity:

• No issues here based on the provided diff.

Suggestions:

• Ensure that the newly injected urlValidationService is actually used for any relevant URL inputs within the JobService (e.g., if jobCreationDto contains any URLs that lead to external resources).
  Review Summary:

The pull request introduces changes to JobService for handling false positive files and adds a new StatisticService method with an associated DTO for fetching job activity data.

Overall, the changes to JobService aim to add more robust validation for false positive file URLs, which is good, but contain a logical flaw. The new statistics endpoint provides valuable functionality, but its current implementation has significant performance concerns due to an inefficient algorithm.

---

src/main/java/com/redhat/sast/api/service/JobService.java

Comments:

• Logical Flaw in shouldUseFalsePositiveFile:
  • Problem: In the condition if (!defaultToUse || job.getKnownFalsePositivesUrl() == null), if defaultToUse is true (meaning useFromDto was true or null) AND job.getKnownFalsePositivesUrl() == null, the method currently returns true. If there is no URL for known false positives, the system cannot technically "use" such a file.
  • Suggestion: If job.getKnownFalsePositivesUrl() is null, the method should logically return false, as there's no file to use. The current logic implies it would try to use a non-existent file if the DTO didn't explicitly say false.
  • Revised Logic Example:

    private boolean shouldUseFalsePositiveFile(Job job, JobCreationDto jobCreationDto) {
        Boolean useFromDto = jobCreationDto.getUseKnownFalsePositiveFile();
        boolean explicitDecision = (useFromDto != null) ? useFromDto : true; // Default to true if not specified
    
        // If the DTO explicitly said NOT to use it, then don't.
        if (!explicitDecision) {
            return false;
        }
    
        // If it was explicitly true or defaulted to true, we must have a URL.
        // If no URL is provided, we cannot use the file.
        if (job.getKnownFalsePositivesUrl() == null) {
            LOGGER.info(
                    "No known false positives URL provided for package '{}'. Setting useKnownFalsePositiveFile to false.",
                    job.getPackageNvr());
            return false;
        }
    
        // Now check accessibility if a URL exists and we're supposed to use it.
        if (urlValidationService.isUrlAccessible(job.getKnownFalsePositivesUrl())) {
            return true;
        }
    
        LOGGER.info(
                "Known false positives file not found for package '{}' at URL: {}. Setting useKnownFalsePositiveFile to false.",
                job.getPackageNvr(),
                job.getKnownFalsePositivesUrl());
        return false;
    }

• Security (SSRF Protection):
  • Problem: The urlValidationService.isUrlAccessible() method is a critical point for security. The PR doesn't show its implementation.
  • Suggestion: Ensure urlValidationService.isUrlAccessible() thoroughly validates the URL to prevent Server-Side Request Forgery (SSRF) attacks. This includes checking the protocol, hostname, and port, disallowing internal/private IPs, preventing redirects to unintended locations, and handling various URL encoding schemes.

---

src/main/java/com/redhat/sast/api/service/StatisticService.java

Comments:

• Performance: Inefficient Algorithm for getJobActivity24h:

  • Problem: The method iterates 24 times, and within each iteration, it performs 4 separate database queries to count jobs for each status. This results in 24 * 4 = 96 database queries for a single API call. This is highly inefficient and will likely lead to performance bottlenecks and increased database load, especially as the system scales or if the API endpoint is frequently accessed.
  • Suggestion: Refactor this method to use a single, more complex database query that aggregates the counts by status and hour for the last 24 hours. Most relational databases support functions like DATE_TRUNC or similar to group data by hour. This would drastically reduce the number of database round trips and improve performance.
  • Example (Conceptual SQL, actual JPA/Hibernate would vary):

    SELECT
        DATE_TRUNC('hour', createdAt) AS hour,
        status,
        COUNT(*)
    FROM job
    WHERE createdAt >= NOW() - INTERVAL '24 hours'
    GROUP BY 1, 2
    ORDER BY 1, 2;

    Then, in Java, process this single result set to construct the List<JobActivityDataPointDto>.

• API Design: Timestamp Format:

  • Problem: The timestamp field in JobActivityDataPointDto is set using hourStart.toString(). LocalDateTime.toString() does not include timezone information, which can lead to ambiguity or incorrect interpretations by API clients, especially when dealing with historical data or systems in different timezones.
  • Suggestion: Use a timezone-aware type like java.time.Instant or java.time.OffsetDateTime for the timestamp. When converting to a string for the API response, use an ISO-8601 format that includes timezone information (e.g., Instant.now().toString() or OffsetDateTime.now().toInstant().toString()). This ensures clarity and correctness for consumers of the API.

---

src/main/java/com/redhat/sast/api/v1/dto/response/JobActivityDataPointDto.java

Comments:

• API Design: Timestamp Format:
  • Problem: (Repeated from StatisticService.java review) The DTO likely uses String for the timestamp field.
  • Suggestion: Consider changing the type of the timestamp field from String to Instant or OffsetDateTime to ensure timezone-aware and unambiguous time representation in the API response. This aligns with best practices for API data types.
    Overall, the changes introduce a new API endpoint and DTO for reporting job activity, which is a common and useful feature. The addition of Jackson date serialization configuration is also a good practice. However, there are a couple of areas that need improvement, particularly regarding type safety and error handling.

---

Review Summary:

The pull request adds a new DTO JobActivityDataPointDto, a new REST endpoint /activity/24h to JobResource to retrieve job activity for the last 24 hours, and configures Jackson to serialize dates as ISO-8601 strings in UTC.

Positive Aspects:

• Clarity: The JobActivityDataPointDto is clear in its purpose.
• Best Practices: Using Lombok for DTOs and explicit @JsonProperty is good. Configuring Jackson for ISO-8601 dates with UTC is also an excellent practice.

Areas for Improvement:

• Timestamp Type Safety: The timestamp field in JobActivityDataPointDto should use a java.time object instead of String to leverage type safety and the new Jackson configuration.
• Exception Handling (Security/Robustness): The exception handling in the new endpoint is too broad and risks exposing internal system details, which is a security vulnerability (information disclosure).

---

File Comments:

src/main/java/com/redhat/sast/api/v1/dto/response/JobActivityDataPointDto.java

• Issue: The timestamp field is a String.
• Suggestion: Change the timestamp field type to java.time.Instant or java.time.OffsetDateTime. This provides type safety, allows proper date/time manipulation, and aligns perfectly with the quarkus.jackson.write-dates-as-timestamps=false configuration. Jackson will automatically serialize these types into ISO-8601 strings as configured.

src/main/java/com/redhat/sast/api/v1/resource/JobResource.java

• Issue: The getJobActivity24h() method catches a generic Exception and exposes the exception message directly in the response body.
• Suggestion (Security & Robustness):
  1. Specific Exception Handling: Avoid catching Exception. Instead, catch more specific exceptions that statisticService.getJobActivity24h() might throw (e.g., custom ServiceException or RuntimeException if no specific checked exceptions are expected).
  2. Information Disclosure: Returning e.getMessage() can leak internal system details. For INTERNAL_SERVER_ERROR, the client should receive a generic, user-friendly message (e.g., "An unexpected error occurred.") while the actual exception details (including stack trace) should be logged securely on the server-side for debugging. Consider implementing a global exception mapper for consistent error responses across the API.

src/main/resources/application.properties

• Comment: The addition of quarkus.jackson.write-dates-as-timestamps=false and quarkus.jackson.timezone=UTC is an excellent practice for consistent and standardized date serialization in JSON responses. This change supports the suggestion made for JobActivityDataPointDto.java to use a java.time type.

[JudeNiroshan]

It would be great to consider the time part as a path param. So, you don't need to create another endpoint for the last 1hr

[ikrispin]

Ok i'll create a dedicated task for that if that's ok @JudeNiroshan

[JudeNiroshan]

What I meant is something as follows.

@GET
@Path("/activity/{hours}/hr") // Dynamic path segment
public Response getJobActivityHours(@PathParam("hours") int hours) {

[JudeNiroshan]

Because of this, we will use the time on our server host. It's better to stick to UTC time so we don't have incorrect times.

[ikrispin]

I see, so we should basically replace all instances of LocalDateTime.now() across the project? What should be the approach?

[JudeNiroshan]

Yes, Please change all the places to make it consistent.

[github-actions]

AI Code Review Skipped: This PR is too large for automated review (diff size exceeds 100KB).

[github-actions]

AI Code Review Skipped: This PR is too large for automated review (diff size exceeds 100KB).

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

Overall, this Pull Request makes a significant positive impact, primarily by migrating from `LocalDateTime` to `Instant` for timestamps across the application. This is a widely recommended best practice that enhances data integrity, consistency, and robustness by using unambiguous, UTC-based time representations.

Here are the key aspects and suggestions:

Key Improvements & Best Practices:

• Consistent LocalDateTime to Instant Migration:
  • Applied to entity fields (createdAt, startedAt, lastUpdatedAt, etc.), configuration calculations (retryCutoffTime, retentionCutoffTime), repository method parameters, service logic, and DTOs.
  • Benefit: Ensures all system-level timestamps are unambiguous, globally comparable, and less prone to time zone-related issues. This is crucial for auditing, scheduling, and distributed environments.
• Jackson Configuration for Instant:
  • Added quarkus.jackson.write-dates-as-timestamps=false and quarkus.jackson.timezone=UTC.
  • Benefit: Ensures Instant objects are serialized as human-readable ISO-8601 strings in UTC, improving API interoperability and consistency.
• Improved Pagination:
  • Added Sort.by("createdAt").descending() to findJobsWithPagination calls.
  • Benefit: Provides predictable and consistent ordering for paginated job listings.
• Enhanced Dependency Injection:
  • Switched from field injection (@Inject) to constructor injection in JobResource.
  • Benefit: Improves testability, maintainability, and clarifies mandatory dependencies.

---

Pressing Issues & Code Suggestions:

1. StatisticService.getJobActivity24h() Performance Bottleneck:

   • Problem: The method currently makes 96 separate database calls (jobRepository.countByStatusInTimeWindow) to fetch hourly job counts. This is inefficient and can lead to performance degradation.
   • Suggestion: Refactor to use a single (or a few) more complex SQL query(s) with GROUP BY clauses on the timestamp (truncated to the hour) and job status. Process the aggregated results in memory to construct the DTOs.

2. Inconsistent Timestamp Type in JobActivityDataPointDto:

   • Problem: The timestamp field in JobActivityDataPointDto is defined as String despite the overall migration to Instant. This defeats the purpose of Instant and introduces potential parsing errors and time zone ambiguities for API consumers.
   • Suggestion: Change the timestamp field from String to java.time.Instant in JobActivityDataPointDto to maintain consistency and leverage proper type safety.

3. Broad Error Handling in JobResource./activity/24h Endpoint:

   • Problem: The try-catch (Exception e) block is too broad, and returning e.getMessage() can leak internal system details, posing a security risk and poor API design.
   • Suggestion: Catch more specific exceptions. Return a generic, user-friendly error message or a specific error code to the client, and log the full exception internally for debugging. Consider implementing a global ExceptionMapper for consistent error handling.

Review Summary:
This pull request introduces a positive and significant change by migrating from LocalDateTime to Instant for timestamp fields and calculations. This is a widely recommended best practice for handling system-level timestamps, as Instant represents an unambiguous point in time, avoiding potential issues related to time zones. The conversions and calculations (e.g., minusSeconds, 86400L for days to seconds) are correctly implemented. This change improves data integrity and robustness.

---

src/main/java/com/redhat/sast/api/config/OshConfiguration.java

• Clarity & Simplicity:
  • The change from LocalDateTime to Instant improves clarity by ensuring that all time calculations are based on an unambiguous point in time, rather than a date-time without a specific time zone. This makes the system's time logic more robust and easier to reason about, especially in distributed environments.
• Correctness:
  • The adjustments for getRetryCutoffTime (minusMinutes to minusSeconds(calculatedBackoffMinutes * 60)) and getRetentionCutoffTime (minusDays to minusSeconds(retryRetentionDays * 86400L)) are correct and necessary to align with Instant's API, which primarily operates with Duration or seconds when modifying a point in time.
  • Using Instant.now().minus(Duration.parse(retryBackoffDuration)) is also correct as Duration can be subtracted directly from an Instant.

src/main/java/com/redhat/sast/api/model/DataArtifact.java
src/main/java/com/redhat/sast/api/model/Job.java

• Clarity & Simplicity:
  • Changing createdAt and startedAt fields from LocalDateTime to Instant is a strong best practice. These fields represent specific moments in time when events occurred, and Instant accurately captures this without ambiguity.
• Data Integrity:
  • Using Instant for persistence (@Column) typically maps to TIMESTAMP WITH TIME ZONE in most JPA implementations and databases, which ensures that the exact global point in time is preserved, regardless of the application or database server's local time zone. This is crucial for audit trails and consistent event ordering.
• Security (Indirect):
  • While not a direct security fix, ensuring unambiguous timestamps (Instant) prevents potential time-based logical flaws that might arise from LocalDateTime's time-zone ambiguity. This can indirectly improve the reliability of features like session expiry, data retention, or event logging, which are important for overall system security.
    This is an excellent set of changes.

Review Summary:
This PR significantly improves the robustness and clarity of time handling within the application by replacing LocalDateTime with Instant for all system-generated timestamps. Using Instant ensures that time-related data points are universally comparable, unambiguous (UTC-based), and less prone to issues with time zones or daylight saving changes. This is a best practice for persistence layers in most modern applications.

---

File: src/main/java/com/redhat/sast/api/model/Job.java

• Comment: Good change. Using Instant for createdAt, startedAt, completedAt, cancelledAt, and lastUpdatedAt ensures these system timestamps are unambiguous and universally comparable (UTC). The use of Instant.now() in PrePersist and PreUpdate hooks is correct.

File: src/main/java/com/redhat/sast/api/model/JobBatch.java

• Comment: Consistent improvement. Instant is the correct choice for submittedAt and lastUpdatedAt to track universal points in time for batch operations. The PrePersist and PreUpdate logic correctly aligns with this type change.

File: src/main/java/com/redhat/sast/api/model/JobBatchDataFlow.java

• Comment: Solid. Migrating createdAt to Instant aligns with best practices for tracking immutable, universal event times.

Review Summary

This pull request introduces a consistent change across several JPA entities, migrating the createdAt timestamp field from LocalDateTime to Instant. This is an excellent change that promotes data consistency and eliminates ambiguity related to time zones. Instant represents a point in time on the UTC timeline, making it ideal for backend storage and inter-system communication, preventing potential issues that LocalDateTime (which lacks time zone information) could introduce in distributed or global environments.

File-specific Comments

src/main/java/com/redhat/sast/api/model/JobBatchDataFlow.java

• Improvement: Changing LocalDateTime to Instant for createdAt is a solid improvement. Instant provides a universal, time-zone-agnostic timestamp, which is best practice for database storage.

src/main/java/com/redhat/sast/api/model/JobBatchRunDefinition.java

• Improvement: Consistent application of Instant for createdAt is a good move for data integrity and global consistency.

src/main/java/com/redhat/sast/api/model/JobMetrics.java

• Improvement: Using Instant for createdAt ensures that metrics timestamps are unambiguous across different time zones and systems.

src/main/java/com/redhat/sast/api/model/JobTokenUsage.java

• Improvement: Standardizing on Instant for createdAt is a good practice, ensuring reliable timestamp comparisons and ordering.

src/main/java/com/redhat/sast/api/model/OshSchedulerCursor.java

• Improvement: This change correctly applies Instant for createdAt, which is crucial for scheduler-related timestamps to avoid time zone-related discrepancies.
  This PR makes a consistent and positive change across multiple model classes, replacing LocalDateTime with Instant for timestamp fields. This is a highly recommended practice.

Review Summary:
This pull request correctly refactors timestamp fields from LocalDateTime to Instant in several model classes. This is an excellent change as Instant represents a point in time on the UTC timeline, eliminating time-zone ambiguities inherent with LocalDateTime when persisting data. This improves data consistency, reliability, and provides a more robust foundation for time-sensitive operations, which is also beneficial from a security perspective for accurate logging and auditing. The changes are clear, simple, and apply a best practice for handling timestamps in Java applications with persistence.

---

File Comments:

• src/main/java/com/redhat/sast/api/model/OshSchedulerCursor.java

  • Comment: Good change. Using Instant for lastSeenTimestamp and updatedAt ensures that these timestamps are stored consistently in UTC, removing any ambiguity related to server time zones. This is a best practice for database persistence.

• src/main/java/com/redhat/sast/api/model/OshUncollectedScan.java

  • Comment: Excellent. Switching to Instant for createdAt and lastAttemptAt is critical for accurate tracking of scan lifecycle events, especially in a distributed environment where time zone consistency is paramount. Instant.now() correctly provides UTC timestamps.

• src/main/java/com/redhat/sast/api/model/PricingModel.java

  • Comment: Appropriate change. effectiveFrom and effectiveTo being Instant ensures that pricing validity periods are unambiguous globally, preventing potential discrepancies caused by time zone differences or daylight saving changes.
    Review Summary:

The changes primarily focus on standardizing time handling from LocalDateTime to Instant across the application, which is a significant improvement for data consistency and handling across different time zones. Additionally, the pagination for jobs has been enhanced with a default sorting order, and a new utility method for counting jobs within a time window has been added. These are positive changes that enhance the robustness, clarity, and utility of the codebase. No security vulnerabilities or glaring performance issues were introduced.

---

File: (Implicit from the first diff block, likely a Model/Entity class)

Improvement:

• Changing LocalDateTime.now() to Instant.now() for effectiveFrom is an excellent practice. Instant represents a point in time on the timeline, independent of any time zone, which is crucial for consistency, especially when data is stored in a database and accessed by services in potentially different time zones. This removes ambiguity that LocalDateTime can introduce.

File: src/main/java/com/redhat/sast/api/repository/DataArtifactRepository.java

Improvement:

• Consistency in time handling: The change from LocalDateTime to Instant for the deleteOlderThan method's cutoffDate parameter aligns perfectly with the improved time handling across the application. This ensures uniformity and correctness when dealing with timestamps.

File: src/main/java/com/redhat/sast/api/repository/JobRepository.java

Improvement:

• Predictable Pagination: Adding Sort.by("createdAt").descending() to all findJobsWithPagination calls is a good enhancement. It ensures a consistent and predictable order for job listings, which improves the user experience and simplifies client-side handling of paginated data.
• Useful Utility Method: The new countByStatusInTimeWindow method is a valuable addition. It encapsulates a common aggregation pattern, making the code cleaner and less error-prone for callers needing to perform such counts. The use of >= startTime and < endTime is a standard and robust way to define time windows.
  This pull request consistently replaces java.time.LocalDateTime with java.time.Instant in several repository method signatures.

Review Summary:
This is a positive and recommended change. Using Instant instead of LocalDateTime for system timestamps, cut-off times, and last-seen cursors significantly improves robustness and clarity. Instant represents a point on the time-line independent of any time zone, which is crucial for preventing ambiguity and ensuring consistent behavior, especially in distributed systems or when interacting with databases. It avoids potential issues related to server time zones and provides a canonical representation of time.

---

File: src/main/java/com/redhat/sast/api/repository/OshRetryStatisticsRepository.java

• Improvement: Replacing LocalDateTime with Instant in countEligibleForRetry method is good. Instant is ideal for representing a globally consistent cutoff time, avoiding time zone issues.

---

File: src/main/java/com/redhat/sast/api/repository/OshSchedulerCursorRepository.java

• Improvement: Changing LocalDateTime to Instant for the timestamp parameter in updateCursor is appropriate. A "last seen timestamp" should be a universal point in time.

---

File: src/main/java/com/redhat/sast/api/repository/OshUncollectedScanRepository.java

• Improvement: Using Instant for cutoffTime in both findRetryableScansWithLock and deleteOlderThan methods is excellent. This ensures that retry eligibility and data retention policies are applied consistently, regardless of the server's local time zone configuration.
  This is a good, straightforward change that improves the reliability and consistency of timestamp handling in the application.

Review Summary:

This pull request consistently replaces LocalDateTime.now() with Instant.now() across several services and repositories. This is a highly recommended change for applications, especially those operating in distributed environments or requiring robust auditing. Instant represents a point in time on the UTC timeline, making timestamps unambiguous and preventing potential issues related to different server time zones or daylight saving changes.

File-specific Comments:

src/main/java/com/redhat/sast/OshUncollectedScanRepository.java

• Improvement: Changing LocalDateTime.now() to Instant.now() here is excellent for ensuring the now parameter, likely an update timestamp, is consistently in UTC, removing ambiguity.

src/main/java/com/redhat/sast/api/service/DataArtifactService.java

• Improvement: The update to use Instant for createdAt in ArtifactCreationInfo and when creating new artifacts is a significant improvement for data consistency and accurate event logging.

src/main/java/com/redhat/sast/api/service/JobService.java

• Improvement: Standardizing startedAt, cancelledAt, completedAt, and lastUpdatedAt to Instant.now() for job lifecycle events is crucial. This ensures that job timing is recorded universally, regardless of where the service is running, which is vital for monitoring, auditing, and debugging.

src/main/java/com/redhat/sast/api/service/PackageService.java

• Improvement: The import change signals a move towards using Instant within this service. Assuming other LocalDateTime.now() usages within this file (not shown in the diff) are also updated to Instant.now(), this is consistent with the overall improvement.

General Suggestion:

• Consistency Check: While this PR addresses several instances, it's good practice to ensure a comprehensive scan for any remaining LocalDateTime.now() calls in the codebase that are intended to represent a universal point in time (e.g., event timestamps, creation/modification dates) and apply the Instant.now() change consistently.
• Database Mapping: Confirm that the underlying database columns for these timestamp fields are appropriately configured to store Instant values (e.g., TIMESTAMP WITH TIME ZONE or handled as UTC TIMESTAMP WITHOUT TIME ZONE by the ORM/DB config). Quarkus/Hibernate typically handles Instant correctly by mapping it to TIMESTAMP WITH TIME ZONE or a similar type, but it's worth a quick check, especially if dealing with legacy schemas.

Review Summary:
The change from `LocalDateTime` to `Instant` is a good improvement, enhancing clarity and robustness by using a time-zone-agnostic representation for timestamps, which is generally recommended for backend services.

The new `getJobActivity24h()` method introduces valuable functionality but has potential performance and data type representation issues that should be addressed.

---

**src/main/java/com/redhat/sast/api/service/JobManageService.java**

*   **No major issues.**
*   **Comment:** The change from `LocalDateTime` to `Instant` for `lastAnalysis` is good. `Instant` is preferred for machine-readable timestamps to avoid time zone ambiguities.

---

**src/main/java/com/redhat/sast/api/service/StatisticService.java**

*   **Issue 1: Potential Performance Bottleneck in `getJobActivity24h()`**
    *   **Problem:** The `getJobActivity24h()` method makes `24 * 4 = 96` separate database calls (`jobRepository.countByStatusInTimeWindow`) to retrieve hourly job counts for the last 24 hours. This can be very inefficient and slow down the API, especially if the `jobRepository` calls are expensive or the database round-trip latency is high.
    *   **Suggestion:** Consider refactoring this to use a more efficient database query. Instead of 96 individual counts, a single (or a few) complex SQL query using `GROUP BY` clauses on the timestamp (truncated to the hour) and job status could fetch all required data in one go. This would drastically reduce database load and improve response times.
        *   *Example (pseudo-SQL):* `SELECT TRUNCATE_TO_HOUR(createdAt), status, COUNT(*) FROM Job WHERE createdAt >= ? AND createdAt < ? GROUP BY TRUNCATE_TO_HOUR(createdAt), status;`
        *   The results can then be processed in memory to construct the `JobActivityDataPointDto` list.

*   **Issue 2: Data Type for Timestamp in `JobActivityDataPointDto`**
    *   **Problem:** In `getJobActivity24h()`, `dataPoint.setTimestamp(hourStart.toString());` converts `Instant` to a `String` for the DTO. While `String` can represent a timestamp, it loses the type safety and explicit temporal context that `Instant` or a similar Java 8 Date/Time API class provides. It also requires parsing if the consumer needs to work with the timestamp programmatically, introducing potential errors.
    *   **Suggestion:** If possible, change the `timestamp` field in `JobActivityDataPointDto` to use `java.time.Instant`. This keeps the data type consistent and avoids unnecessary string conversions, making the DTO clearer and more robust for consumers. If the API contract specifically requires a String, ensure it's well-documented and consistent.

*   **No major issues:** The change from `LocalDateTime.now()` to `Instant.now()` in `getOshScanSummary` is consistent with the other `Instant` usage and is a good practice.

---

**src/main/java/com/redhat/sast/api/service/osh/OshRetryService.java**

*   **No changes visible in the provided diff snippet.**

This PR introduces a positive change by migrating from LocalDateTime to Instant for handling timestamps related to retry cutoffs, cleanup operations, and status reporting.

Review Summary:
This is an excellent change. Replacing LocalDateTime with Instant is generally a best practice for managing absolute points in time, especially in backend services, scheduling, and distributed systems. Instant represents a point on the time-line in UTC, which avoids timezone-related ambiguities and potential bugs that LocalDateTime (which is timezone-agnostic) can introduce. The changes are consistent and correctly applied across the affected code.

---

File: src/main/java/com/redhat/sast/api/service/osh/OshRetryService.java

• Improvement: The migration from LocalDateTime to Instant for cutoffTime and startTime is a good practice. Instant accurately represents a specific moment in time regardless of the system's local timezone, which is crucial for reliable scheduling, retry logic, and cleanup operations.
• Clarity/Simplicity: Using Instant clarifies that these timestamps refer to an absolute point in time. The corresponding change for calculating duration with Duration.between(startTime, Instant.now()) is also correct and consistent.

File: src/main/java/com/redhat/sast/api/startup/OshScheduler.java

• Improvement: Similar to OshRetryService, the switch to Instant for OshScheduler is beneficial for consistency and correctness when dealing with scheduled tasks and their associated timestamps.
  Review Summary:

The pull request primarily focuses on improving timestamp handling by migrating from LocalDateTime to Instant across several DTOs and a service method. This is a positive change, as Instant provides a clear, unambiguous, and timezone-agnostic representation of a point in time, which is generally preferred for backend storage and API communication.

However, a significant inconsistency is introduced with the new JobActivityDataPointDto, where the timestamp field is defined as a String instead of Instant. This deviates from the beneficial changes made elsewhere in the PR and can lead to parsing issues, lack of type safety, and potential timezone ambiguities for API consumers.

---

File Comments:

src/main/java/com/redhat/sast/core/service/impl/OpenShiftHistoryServiceImpl.java

• Comment: Good change. Using Instant.now() for recording timestamps is more robust and avoids timezone ambiguities compared to LocalDateTime.now().

src/main/java/com/redhat/sast/api/v1/dto/response/DashboardSummaryDto.java

• Comment: Good change. Consistent with best practices, using Instant for API response timestamps is clearer and more globally understandable.

src/main/java/com/redhat/sast/api/v1/dto/response/HealthResponseDto.java

• Comment: Good change. Aligning the health endpoint's timestamp to Instant improves consistency and clarity, just like other DTOs.

src/main/java/com/redhat/sast/api/v1/dto/response/JobActivityDataPointDto.java

• Issue: Inconsistency and potential for errors. The timestamp field is defined as a String, which is inconsistent with the beneficial LocalDateTime to Instant migration seen in other files within this PR. Using String for timestamps in DTOs can lead to parsing issues, requires client-side validation, and makes it harder to ensure consistent date formats.
• Suggestion: Change the timestamp field from String to java.time.Instant. This will ensure consistency across the API, leverage proper date/time serialization/deserialization by libraries like Jackson, and provide a more robust and unambiguous timestamp for consumers.
  This is a good and recommended change.

Review Summary:

The pull request consistently replaces LocalDateTime with Instant across several DTOs. This is an excellent improvement for handling timestamps in an API, promoting clarity, consistency, and avoiding time zone ambiguities.

---

File: src/main/java/com/redhat/sast/api/v1/dto/response/JobBatchResponseDto.java
File: src/main/java/com/redhat/sast/api/v1/dto/response/JobResponseDto.java
File: src/main/java/com/redhat/sast/api/v1/dto/response/OshScanStatusDto.java
File: src/main/java/com/redhat/sast/api/v1/dto/response/PackageSummaryDto.java

• Comment: Replacing LocalDateTime with Instant for timestamps is a very good practice for API DTOs. Instant represents a point in time on the global timeline, independent of any time zone, which is crucial for consistency and avoiding potential errors when clients are in different geographical locations. LocalDateTime lacks time zone information, making it ambiguous and prone to misinterpretation if not handled carefully with explicit time zone conversions. This change improves data integrity and clarity.
  Review Summary:
  The changes consistently switch from LocalDateTime to Instant for various timestamp fields in DTOs. This is an excellent improvement. Instant represents a specific point in time on the timeline, without any time zone information, making it ideal for system-level timestamps, database storage, and inter-service communication. This change reduces ambiguity, potential time zone issues, and generally leads to more robust date/time handling. No issues found, this is a good change.

---

File: src/main/java/com/redhat/sast/api/v1/dto/response/analysis/AnalysisSummaryResponseDto.java

• Comment: Changing lastAnalysisDate from LocalDateTime to Instant is a good practice. Instant is better for representing a point in time that is independent of any time zone.

File: src/main/java/com/redhat/sast/api/v1/dto/response/admin/OshRetryQueueResponseDto.java

• Comment: Consistent change from LocalDateTime to Instant for timestamp field and its initialization. This improves clarity and reduces potential timezone-related issues for system timestamps.

File: src/main/java/com/redhat/sast/api/v1/dto/response/admin/OshRetryStatisticsResponseDto.java

• Comment: Consistent change from LocalDateTime to Instant for oldestScan, newestScan, timestamp fields, and their initializations. This is a solid improvement for representing unambiguous points in time.
  Review Summary:

The changes primarily focus on improving timestamp handling and dependency injection practices, along with adding a new API endpoint. The shift from LocalDateTime to Instant is a good practice for representing global timestamps. Moving to constructor injection in JobResource significantly improves testability and code maintainability. The new /activity/24h endpoint is well-scoped, but its error handling could be more robust and secure.

---

File: OshStatusResponseDto.java

• Issue: No major issues.
• Suggestion: Using Instant instead of LocalDateTime for timestamps is a good and consistent practice, avoiding potential timezone ambiguities. This is a clear improvement.

---

File: JobResource.java

• Issue 1: Dependency Injection Method

  • Comment: The change from @Inject (field injection) to constructor-based injection is an excellent improvement. Constructor injection makes dependencies explicit, mandatory, and enhances testability.
  • Suggestion: Keep up the good work; this is a best practice.

• Issue 2: Error Handling for /activity/24h Endpoint

  • Comment: The try-catch (Exception e) block is too broad, and returning e.getMessage() can leak internal system details, which is a security risk and poor API design for external consumers.
  • Suggestion:
    1. Catch more specific exceptions if possible (e.g., JobActivityRetrievalException or ServiceUnavailableException).
    2. Return a generic, user-friendly error message or a specific error code instead of e.getMessage() to avoid information leakage. Log the full exception internally for debugging.
    3. Consider using an ExceptionMapper for a more consistent and centralized error handling strategy across the API.

---

File: OshAdminResource.java

• Issue: No major issues.
• Suggestion: Consistent use of Instant for timestamps across DTOs and resources is a good and consistent practice. This is a clear improvement.
  Overall, this is an excellent set of changes that significantly improves the robustness and clarity of date and time handling within the application.

Review Summary:

The changes consistently transition from LocalDateTime to Instant for timestamps in the OshAdminResource and its related DTOs (CleanupResult, PollResult). This is further complemented by configuring Jackson to serialize dates as ISO-8601 strings in UTC.

Key Benefits:

• Clarity and Accuracy: Instant represents a point in time on the timeline, independent of any time zone, which eliminates ambiguity inherent with LocalDateTime. This is crucial for distributed systems, logging, and auditing.
• Consistency: Standardizing on Instant and ensuring all serialization is done in UTC prevents potential time zone-related bugs and inconsistencies.
• Interoperability: Serializing dates as ISO-8601 strings is the widely accepted standard for REST APIs, making the output more readable and easier for other systems to consume.
• Security: Unambiguous timestamps in UTC are vital for accurate logging, auditing, and incident investigation. They eliminate potential for misinterpretation of events due to varying time zones.

There are no issues found with these changes; they are purely improvements.

File-specific Comments:

src/main/java/com/example/OshAdminResource.java

• Improvement: Replacing LocalDateTime.now() with Instant.now() and updating the DTOs (CleanupResult, PollResult) to use Instant is a strong best practice. Instant provides a clear, unambiguous point in time on the UTC timeline, avoiding potential issues with time zone interpretation or inconsistencies that can arise with LocalDateTime. This makes the application's timestamps more reliable, especially in a distributed environment or when interacting with external systems.

src/main/resources/application.properties

• Improvement: The addition of quarkus.jackson.write-dates-as-timestamps=false and quarkus.jackson.timezone=UTC is an excellent configuration change.
  • write-dates-as-timestamps=false: Ensures that java.time objects are serialized into human-readable ISO-8601 strings (e.g., "2023-10-27T10:00:00Z") rather than numeric timestamps (milliseconds since epoch). This significantly improves API readability and interoperability.
  • timezone=UTC: Explicitly sets the default time zone for Jackson serialization to UTC. This reinforces the use of Instant throughout the code and ensures consistent timestamp output, preventing potential discrepancies caused by differing default JVM time zones. This is a robust approach for maintaining data integrity and consistency.
    Review Summary:

This PR is a positive refactoring, primarily switching from LocalDateTime to Instant for time-related operations in test files. This change improves the clarity and robustness of time handling, as Instant represents a specific point on the timeline independent of time zones, which is generally preferred for system-level timestamps and calculations involving durations.

---

File Comments:

src/test/java/com/redhat/sast/api/config/OshConfigurationTest.java

• Clarity/Simplicity: The change from LocalDateTime to Instant for handling cutoff and retention times is a good improvement. Instant is better suited for representing a specific moment in time, especially when dealing with durations and comparisons, as it avoids potential ambiguities related to time zones or Daylight Saving Time that LocalDateTime might introduce if not handled carefully.
• Suggestions: None. The change is straightforward and beneficial.

src/test/java/com/redhat/sast/api/repository/OshSchedulerCursorRepositoryTest.java

• Clarity/Simplicity: Consistent with the previous file, using Instant instead of LocalDateTime for repository-related time values is a good practice. It ensures that timestamps are stored and processed as universal points in time, improving reliability and avoiding potential inconsistencies across different environments or time zones.
• Suggestions: None. The change is straightforward and beneficial.

• File: OshSchedulerCursorRepositoryTest

Review Summary:
This change is a very good improvement. Migrating from LocalDateTime to Instant for storing timestamps is a best practice. Instant represents a specific point in time on the timeline, independent of time zones, which greatly reduces potential bugs and inconsistencies related to time zone conversions. The test updates are consistent with this change and ensure the functionality remains correct.

Issue Comments:

• None. The changes are positive and well-executed.
  File: [filename - assuming this is from a .java file, e.g., CursorUpdaterService.java] (Please provide the file name if known)

Review Comment:

This is a good change. Using Instant.now() for a persistent_token cursor is generally preferred over LocalDateTime.now().

• Reasoning: Instant represents a point in time on the global timeline, independent of any time-zone. LocalDateTime represents a date and time without a time-zone, which can lead to ambiguity or inconsistencies if the server's time-zone changes or if the application runs across multiple time-zones. For persistence (like a database cursor), Instant ensures global consistency.

[JudeNiroshan]

Shouldn't we simply use Instant.now()? It guarantees that we will show the latest data for sure.

[JudeNiroshan]

What I meant is something as follows.

@GET
@Path("/activity/{hours}/hr") // Dynamic path segment
public Response getJobActivityHours(@PathParam("hours") int hours) {

[JudeNiroshan]

What's the importance of this flag? Please add a comment

[JudeNiroshan]

I would not create a method strictly for 24h time period. Maybe you could think about a generic solution that can retrieve any number of hours if you'd like.