camunda / camunda-bpm-platform

Flexible framework for workflow and decision automation with BPMN and DMN. Integration with Quarkus, Spring, Spring Boot, CDI.
https://camunda.com/
Apache License 2.0
4.11k stars 1.55k forks source link

Set Job Retries Async (POST) throws NPE when request body contains createTimes or dueDates #3185

Closed flok32 closed 1 year ago

flok32 commented 1 year ago

Environment (Required on creation)

7.18.0, engine-rest

Description (Required on creation; please attach any relevant screenshots, stacktraces, log files, etc. to the ticket)

When calling the rest endpoint /job/retries (Set Job Retries Async (POST)) with a request that contains a "createTimes" condition or a "dueDates" condition, a NPE is thrown:

10:01:48.383 [grizzly-http-server-0] WARN  o.camunda.bpm.engine.rest.exception - ENGINE-REST-HTTP500 java.lang.NullPointerException: Cannot invoke "com.fasterxml.jackson.databind.ObjectMapper.readValue(String, java.lang.Class)" because "this.objectMapper" is null
    at org.camunda.bpm.engine.rest.dto.converter.JacksonAwareStringToTypeConverter.mapToType(JacksonAwareStringToTypeConverter.java:42)
    at org.camunda.bpm.engine.rest.dto.converter.DateConverter.convertQueryParameterToType(DateConverter.java:33)
    at org.camunda.bpm.engine.rest.dto.runtime.JobQueryDto$ApplyDates.run(JobQueryDto.java:257)
    at org.camunda.bpm.engine.rest.dto.runtime.JobQueryDto.applyFilters(JobQueryDto.java:390)
    at org.camunda.bpm.engine.rest.dto.runtime.JobQueryDto.applyFilters(JobQueryDto.java:1)
    at org.camunda.bpm.engine.rest.dto.AbstractQueryDto.toQuery(AbstractQueryDto.java:104)
    at org.camunda.bpm.engine.rest.impl.JobRestServiceImpl.setRetries(JobRestServiceImpl.java:115)

Reason for the NPE: The objectmapper ist null. JacksonAwareStringToTypeConverter.java:42:

      return objectMapper.readValue(value, typeClass);

Reason for objectMaper==null: Different from handling of queries the objectMapper ist not set on JobQueryDto in setRetries-Method. JobRestServiceImpl.java:99ff:

  public BatchDto setRetries(SetJobRetriesDto setJobRetriesDto) {
    try {
      EnsureUtil.ensureNotNull("setJobRetriesDto", setJobRetriesDto);
      EnsureUtil.ensureNotNull("retries", setJobRetriesDto.getRetries());
    } catch (NullValueException e) {
      throw new InvalidRequestException(Status.BAD_REQUEST, e.getMessage());
    }
    JobQuery jobQuery = null;
    if (setJobRetriesDto.getJobQuery() != null) {
      jobQuery = setJobRetriesDto.getJobQuery().toQuery(getProcessEngine());     //<-- here the objectmapper is not set on the JobQueryDto returned from getJobQuery()
    }

Comparision with queryJobsCount, JobRestServiceImpl.java:86ff:

  public CountResultDto queryJobsCount(JobQueryDto queryDto) {
    ProcessEngine engine = getProcessEngine();
    queryDto.setObjectMapper(getObjectMapper());
    JobQuery query = queryDto.toQuery(engine);

Steps to reproduce (Required on creation)

POST to /job/retries with body
{ "jobQuery":
    { "jobDefinitionId": "<some process instance id>"
    , "createTimes":
        [ { "operator": "lt"
          , "value": "2022-12-15T10:45:00.000+0100"
          }
        ]
    }
, "retries": 1
}

Observed Behavior (Required on creation)

NullPointerException is thrown

Expected behavior (Required on creation)

NullPointerException is not thrown, Retry Count for the Jobs is set

Root Cause (Required on prioritization)

Solution Ideas

Set objectMapper on JobQueryDto returned from getJobQuery().

JobRestServiceImpl.java:99ff:

  public BatchDto setRetries(SetJobRetriesDto setJobRetriesDto) {
    try {
      EnsureUtil.ensureNotNull("setJobRetriesDto", setJobRetriesDto);
      EnsureUtil.ensureNotNull("retries", setJobRetriesDto.getRetries());
    } catch (NullValueException e) {
      throw new InvalidRequestException(Status.BAD_REQUEST, e.getMessage());
    }
    JobQuery jobQuery = null;
    if (setJobRetriesDto.getJobQuery() != null) {
      JobQueryDto jobQueryDto = setJobRetriesDto.getJobQuery();
      jobQueryDto.setObjectMapper(getObjectMapper());
      jobQuery = jobQueryDto.toQuery(getProcessEngine());
    }

I will try to provide a Pull-Request for the fix.

Hints

Links

Camunda Forum: https://forum.camunda.io/t/issue-with-set-job-retries-async-post-while-using-dates/31843

Breakdown

Dev2QA handover

flok32 commented 1 year ago

Pull Request: https://github.com/camunda/camunda-bpm-platform/pull/3187

yanavasileva commented 1 year ago

Hello @flok32,

Thank you for reaching out to us with this and already prepared contribution for it. I will have a look and get back to you by the end of next week.

Best regards, Yana

yanavasileva commented 1 year ago

Hi @flok32,

I was able to confirm the bug. Thank you for taking the time to raise a pull request about it. I triggered the CI for it, but it looks already good. I will provide you feedback on the code next week, the product still supports and it's build with JDK 8. check the style and so on. I just wanted to give you already heads up about what's next.

Best regards, Yana

yanavasileva commented 1 year ago

Hi @flok32,

I have merged the contribution to the code base. It will be shipped with 7.19.0 scheduled for next month. Once again, thank you for your contribution and see you in the next one.

Best regards, Yana