diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java index 3dbb5962713de..12987d3a17027 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java @@ -328,49 +328,63 @@ public void onFailure(Exception e) { public void updateJob(UpdateJobAction.Request request, ActionListener actionListener) { - ActionListener postUpdateAction; + Runnable doUpdate = () -> { + jobConfigProvider.updateJobWithValidation(request.getJobId(), request.getJobUpdate(), maxModelMemoryLimit, + this::validate, ActionListener.wrap( + updatedJob -> postJobUpdate(request, updatedJob, actionListener), + actionListener::onFailure + )); + }; - // Autodetect must be updated if the fields that the C++ uses are changed - if (request.getJobUpdate().isAutodetectProcessUpdate()) { - postUpdateAction = ActionListener.wrap( - updatedJob -> { - JobUpdate jobUpdate = request.getJobUpdate(); - if (isJobOpen(clusterService.state(), request.getJobId())) { - updateJobProcessNotifier.submitJobUpdate(UpdateParams.fromJobUpdate(jobUpdate), ActionListener.wrap( - isUpdated -> { - if (isUpdated) { - auditJobUpdatedIfNotInternal(request); - } - }, e -> { - // No need to do anything - } - )); + if (request.getJobUpdate().getGroups() != null && request.getJobUpdate().getGroups().isEmpty() == false) { + + // check the new groups are not job Ids + jobConfigProvider.jobIdMatches(request.getJobUpdate().getGroups(), ActionListener.wrap( + matchingIds -> { + if (matchingIds.isEmpty()) { + doUpdate.run(); + } else { + actionListener.onFailure(new ResourceAlreadyExistsException( + Messages.getMessage(Messages.JOB_AND_GROUP_NAMES_MUST_BE_UNIQUE, matchingIds.get(0)))); } - actionListener.onResponse(new PutJobAction.Response(updatedJob)); }, actionListener::onFailure - ); + )); } else { - postUpdateAction = ActionListener.wrap(job -> { - logger.debug("[{}] No process update required for job update: {}", () -> request.getJobId(), () -> { - try { - XContentBuilder jsonBuilder = XContentFactory.jsonBuilder(); - request.getJobUpdate().toXContent(jsonBuilder, ToXContent.EMPTY_PARAMS); - return Strings.toString(jsonBuilder); - } catch (IOException e) { - return "(unprintable due to " + e.getMessage() + ")"; + doUpdate.run(); + } + } + + private void postJobUpdate(UpdateJobAction.Request request, Job updatedJob, ActionListener actionListener) { + // Autodetect must be updated if the fields that the C++ uses are changed + if (request.getJobUpdate().isAutodetectProcessUpdate()) { + JobUpdate jobUpdate = request.getJobUpdate(); + if (isJobOpen(clusterService.state(), request.getJobId())) { + updateJobProcessNotifier.submitJobUpdate(UpdateParams.fromJobUpdate(jobUpdate), ActionListener.wrap( + isUpdated -> { + if (isUpdated) { + auditJobUpdatedIfNotInternal(request); } - }); + }, e -> { + // No need to do anything + } + )); + } + } else { + logger.debug("[{}] No process update required for job update: {}", () -> request.getJobId(), () -> { + try { + XContentBuilder jsonBuilder = XContentFactory.jsonBuilder(); + request.getJobUpdate().toXContent(jsonBuilder, ToXContent.EMPTY_PARAMS); + return Strings.toString(jsonBuilder); + } catch (IOException e) { + return "(unprintable due to " + e.getMessage() + ")"; + } + }); - auditJobUpdatedIfNotInternal(request); - actionListener.onResponse(new PutJobAction.Response(job)); - }, - actionListener::onFailure); + auditJobUpdatedIfNotInternal(request); } - - jobConfigProvider.updateJobWithValidation(request.getJobId(), request.getJobUpdate(), maxModelMemoryLimit, - this::validate, postUpdateAction); + actionListener.onResponse(new PutJobAction.Response(updatedJob)); } private void validate(Job job, JobUpdate jobUpdate, ActionListener handler) { diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml index 285ebfc1cf9e5..f65406a25cabe 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml @@ -397,6 +397,28 @@ "description":"Can't update all description" } + - do: + xpack.ml.put_job: + job_id: job-crud-update-group-name-clash + body: > + { + "analysis_config" : { + "bucket_span": "1h", + "detectors" :[{"function":"metric","field_name":"responsetime","by_field_name":"airline"}] + }, + "data_description" : { + } + } + + - do: + catch: "/job and group names must be unique/" + xpack.ml.update_job: + job_id: jobs-crud-update-job + body: > + { + "groups": ["job-crud-update-group-name-clash"] + } + --- "Test cannot decrease model_memory_limit below current usage": - skip: