Skip to content

Commit

Permalink
Merge pull request #296 from rundeck/issue/295
Browse files Browse the repository at this point in the history
Fix #295 URI too large error for jobs bulk delete
  • Loading branch information
gschueler authored Mar 19, 2020
2 parents 8a8586b + 607ad6b commit 995859e
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,11 @@ Call<ImportResult> loadJobs(
Call<DeleteJobsResult> deleteJobs(
@Query("ids") List<String> ids
);
@Headers("Accept: application/json")
@POST("jobs/delete")
Call<DeleteJobsResult> deleteJobsBulk(
@Body BulkJobDelete body
);

@Headers("Accept: application/json")
@GET("projects")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package org.rundeck.client.api.model;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonInclude;
import lombok.Data;

import java.util.List;

@JsonIgnoreProperties(ignoreUnknown = true)
@JsonInclude(JsonInclude.Include.NON_NULL)
@Data
public class BulkJobDelete {

private List<String> ids;

@JsonCreator
public BulkJobDelete(final List<String> ids) {
this.ids = ids;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ public List<Object> getSubCommands() {
@CommandLineInterface(application = "purge") interface Purge extends JobPurgeOptions, ListOpts {
@Option(longName = "confirm", shortName = "y", description = "Force confirmation of delete request.")
boolean isConfirm();

@Option(longName = "batch", shortName = "b", description = "Batch size if there are many IDs")
Integer getBatchSize();
boolean isBatchSize();

@Option(longName = "max", shortName = "m", description = "Maximum number of jobs to delete")
Integer getMax();
boolean isMax();
}

@Command(description = "Delete jobs matching the query parameters. Optionally save the definitions to a file " +
Expand Down Expand Up @@ -108,31 +116,40 @@ public boolean purge(Purge options, CommandOutput output) throws IOException, In
if (options.isFile()) {
list(options, output);
}
int idsSize = ids.size();
int idsToDelete = options.isMax() ? Math.min(idsSize, options.getMax()) : idsSize;
if (!options.isConfirm()) {
//request confirmation
if (null == System.console()) {
output.error("No user interaction available. Use --confirm to confirm purge without user interaction");
output.warning(String.format("Not deleting %d jobs", ids.size()));
output.warning(String.format("Not deleting %d jobs", idsToDelete));
return false;
}
String s = System.console().readLine("Really delete %d Jobs? (y/N) ", ids.size());
String s = System.console().readLine("Really delete %d Jobs? (y/N) ", idsToDelete);

if (!"y".equals(s)) {
output.warning(String.format("Not deleting %d jobs", ids.size()));
output.warning(String.format("Not deleting %d jobs", idsToDelete));
return false;
}
}

final List<String> finalIds = ids;
DeleteJobsResult deletedJobs = apiCall(api -> api.deleteJobs(finalIds));

if (deletedJobs.isAllsuccessful()) {
output.info(String.format("%d Jobs were deleted%n", deletedJobs.getRequestCount()));
return true;
int batch = options.isBatchSize() ? Math.min(idsToDelete, options.getBatchSize()) : idsToDelete;
int total=0;
for (int i = 0; i < idsToDelete; ) {
int batchToUse = Math.min(batch, idsToDelete - total);
final List<String> finalIds = new ArrayList<>(batchToUse);
finalIds.addAll(ids.subList(i, i + batchToUse));
DeleteJobsResult deletedJobs = apiCall(api -> api.deleteJobsBulk(new BulkJobDelete(finalIds)));
if(!deletedJobs.isAllsuccessful()){
output.error(String.format("Failed to delete %d Jobs%n", deletedJobs.getFailed().size()));
output.output(deletedJobs.getFailed().stream().map(DeleteJob::toBasicString).collect(Collectors.toList()));
return false;
}
total += finalIds.size();
i += batchToUse;
}
output.error(String.format("Failed to delete %d Jobs%n", deletedJobs.getFailed().size()));
output.output(deletedJobs.getFailed().stream().map(DeleteJob::toBasicString).collect(Collectors.toList()));
return false;

output.info(String.format("%d Jobs were deleted%n", total));
return true;
}

@CommandLineInterface(application = "load") interface Load extends JobLoadOptions, VerboseOption {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ class JobsSpec extends Specification {
then:
1 * api.listJobs('ProjectName', job, group, jobexact, groupexact) >>
Calls.response([new JobItem(id: 'fakeid')])
1 * api.deleteJobs(['fakeid']) >> Calls.response(new DeleteJobsResult(allsuccessful: true))
1 * api.deleteJobsBulk({it.ids==['fakeid']}) >> Calls.response(new DeleteJobsResult(allsuccessful: true))
0 * api._(*_)
result

Expand All @@ -224,6 +224,71 @@ class JobsSpec extends Specification {
null | null | 'a' | 'b/c'
null | null | null | 'b/c'
}
@Unroll
def "job purge with with batchsize"() {
given:
def api = Mock(RundeckApi)

def opts = Mock(Jobs.Purge) {
getProject() >> 'ProjectName'
isJob() >> (job != null)
getJob() >> job
isConfirm() >> true
getBatchSize()>>batch
isBatchSize()>>(batch>0)
getMax()>>max
isMax()>>(max>0)
}
def retrofit = new Retrofit.Builder().baseUrl('http://example.com/fake/').build()
def client = new Client(api, retrofit, null, null, 17, true, null)
def hasclient = Mock(RdApp) {
getClient() >> client
}
Jobs jobs = new Jobs(hasclient)
def out = Mock(CommandOutput)
def iter=0
when:
def result = jobs.purge(opts, out)

then:
1 * api.listJobs('ProjectName', job, null, null, null) >> {
Calls.response((1..total).collect{new JobItem(id: "fakeid_$it")})
}
(expect.size()) * api.deleteJobsBulk({it.ids.size()==expect[iter++]}) >> Calls.response(new DeleteJobsResult(allsuccessful: true))
0 * api._(*_)
result

where:
job | batch | total | max || expect
'a' | -1 | 5 | -1 || [5]
'a' | 1 | 5 | -1 || [1, 1, 1, 1, 1,]
'a' | 2 | 5 | -1 || [2, 2, 1,]
'a' | 3 | 5 | -1 || [3, 2]
'a' | 4 | 5 | -1 || [4, 1]
'a' | 5 | 5 | -1 || [5]
'a' | 6 | 5 | -1 || [5]
'a' | 99 | 5 | -1 || [5]

'a' | -1 | 5 | 99 || [5]
'a' | -1 | 5 | 5 || [5]
'a' | -1 | 5 | 4 || [4]
'a' | -1 | 5 | 1 || [1]

'a' | 1 | 5 | 5 || [1, 1, 1, 1, 1,]
'a' | 1 | 5 | 4 || [1, 1, 1, 1,]
'a' | 1 | 5 | 1 || [1,]

'a' | 2 | 5 | 5 || [2, 2, 1,]
'a' | 2 | 5 | 4 || [2, 2,]
'a' | 2 | 5 | 3 || [2, 1,]
'a' | 2 | 5 | 1 || [1,]

'a' | 3 | 5 | 99 || [3, 2]
'a' | 3 | 5 | 5 || [3, 2]
'a' | 3 | 5 | 3 || [3]

'a' | 99 | 5 | 99 || [5]
}

def "job purge invalid input"() {
given:
Expand Down

0 comments on commit 995859e

Please sign in to comment.