Skip to content

Commit

Permalink
Enable XLint warnings for ML (#44285)
Browse files Browse the repository at this point in the history
Removes the warning suppression -Xlint:-deprecation,-rawtypes,-serial,-try,-unchecked.
Many warnings were unchecked warnings in the test code often because of the use of mocks. 
These are suppressed with @SuppressWarning
  • Loading branch information
davidkyle authored Jul 15, 2019
1 parent d70e36d commit 23943ae
Show file tree
Hide file tree
Showing 32 changed files with 86 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public static Set<String> unallocatedJobIds(@Nullable PersistentTasksCustomMetaD
* @param nodes The cluster nodes
* @return Unallocated job tasks
*/
public static Collection<PersistentTasksCustomMetaData.PersistentTask> unallocatedJobTasks(
public static Collection<PersistentTasksCustomMetaData.PersistentTask<?>> unallocatedJobTasks(
@Nullable PersistentTasksCustomMetaData tasks,
DiscoveryNodes nodes) {
if (tasks == null) {
Expand Down Expand Up @@ -247,7 +247,7 @@ public static Set<String> unallocatedDatafeedIds(@Nullable PersistentTasksCustom
* @param nodes The cluster nodes
* @return Unallocated datafeed tasks
*/
public static Collection<PersistentTasksCustomMetaData.PersistentTask> unallocatedDatafeedTasks(
public static Collection<PersistentTasksCustomMetaData.PersistentTask<?>> unallocatedDatafeedTasks(
@Nullable PersistentTasksCustomMetaData tasks,
DiscoveryNodes nodes) {
if (tasks == null) {
Expand Down
3 changes: 0 additions & 3 deletions x-pack/plugin/ml/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ bundlePlugin {
exclude 'platform/licenses/**'
}

compileJava.options.compilerArgs << "-Xlint:-deprecation,-rawtypes,-serial,-try,-unchecked"
compileTestJava.options.compilerArgs << "-Xlint:-deprecation,-rawtypes,-serial,-try,-unchecked"

dependencies {
compileOnly project(':modules:lang-painless:spi')
compileOnly project(path: xpackModule('core'), configuration: 'default')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,8 @@ public static PersistentTasksCustomMetaData rewritePersistentTaskParams(Map<Stri
PersistentTasksCustomMetaData currentTasks,
DiscoveryNodes nodes) {

Collection<PersistentTasksCustomMetaData.PersistentTask> unallocatedJobTasks = MlTasks.unallocatedJobTasks(currentTasks, nodes);
Collection<PersistentTasksCustomMetaData.PersistentTask> unallocatedDatafeedsTasks =
Collection<PersistentTasksCustomMetaData.PersistentTask<?>> unallocatedJobTasks = MlTasks.unallocatedJobTasks(currentTasks, nodes);
Collection<PersistentTasksCustomMetaData.PersistentTask<?>> unallocatedDatafeedsTasks =
MlTasks.unallocatedDatafeedTasks(currentTasks, nodes);

if (unallocatedJobTasks.isEmpty() && unallocatedDatafeedsTasks.isEmpty()) {
Expand All @@ -304,7 +304,7 @@ public static PersistentTasksCustomMetaData rewritePersistentTaskParams(Map<Stri

PersistentTasksCustomMetaData.Builder taskBuilder = PersistentTasksCustomMetaData.builder(currentTasks);

for (PersistentTasksCustomMetaData.PersistentTask jobTask : unallocatedJobTasks) {
for (PersistentTasksCustomMetaData.PersistentTask<?> jobTask : unallocatedJobTasks) {
OpenJobAction.JobParams originalParams = (OpenJobAction.JobParams) jobTask.getParams();
if (originalParams.getJob() == null) {
Job job = jobs.get(originalParams.getJobId());
Expand All @@ -325,7 +325,7 @@ public static PersistentTasksCustomMetaData rewritePersistentTaskParams(Map<Stri
}
}

for (PersistentTasksCustomMetaData.PersistentTask datafeedTask : unallocatedDatafeedsTasks) {
for (PersistentTasksCustomMetaData.PersistentTask<?> datafeedTask : unallocatedDatafeedsTasks) {
StartDatafeedAction.DatafeedParams originalParams = (StartDatafeedAction.DatafeedParams) datafeedTask.getParams();

if (originalParams.getJobId() == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ private static Result<ModelSnapshot> applyUpdate(UpdateModelSnapshotAction.Reque
if (request.getRetain() != null) {
updatedSnapshotBuilder.setRetain(request.getRetain());
}
return new Result(target.index, updatedSnapshotBuilder.build());
return new Result<>(target.index, updatedSnapshotBuilder.build());
}

private void indexModelSnapshot(Result<ModelSnapshot> modelSnapshot, Consumer<Boolean> handler, Consumer<Exception> errorHandler) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public static void create(Client client,
);
return;
}
final List<ValuesSourceAggregationBuilder> flattenedAggs = new ArrayList<>();
final List<ValuesSourceAggregationBuilder<?, ?>> flattenedAggs = new ArrayList<>();
flattenAggregations(datafeed.getParsedAggregations(xContentRegistry)
.getAggregatorFactories(), datafeedHistogramAggregation, flattenedAggs);

Expand Down Expand Up @@ -148,7 +148,7 @@ private static boolean validInterval(long datafeedInterval, ParsedRollupCaps rol

private static void flattenAggregations(final Collection<AggregationBuilder> datafeedAggregations,
final AggregationBuilder datafeedHistogramAggregation,
final List<ValuesSourceAggregationBuilder> flattenedAggregations) {
final List<ValuesSourceAggregationBuilder<?, ?>> flattenedAggregations) {
for (AggregationBuilder aggregationBuilder : datafeedAggregations) {
if (aggregationBuilder.equals(datafeedHistogramAggregation) == false) {
flattenedAggregations.add((ValuesSourceAggregationBuilder)aggregationBuilder);
Expand All @@ -157,8 +157,8 @@ private static void flattenAggregations(final Collection<AggregationBuilder> dat
}
}

private static boolean hasAggregations(ParsedRollupCaps rollupCaps, List<ValuesSourceAggregationBuilder> datafeedAggregations) {
for (ValuesSourceAggregationBuilder aggregationBuilder : datafeedAggregations) {
private static boolean hasAggregations(ParsedRollupCaps rollupCaps, List<ValuesSourceAggregationBuilder<?,?>> datafeedAggregations) {
for (ValuesSourceAggregationBuilder<?,?> aggregationBuilder : datafeedAggregations) {
String type = aggregationBuilder.getType();
String field = aggregationBuilder.field();
if (aggregationBuilder instanceof TermsAggregationBuilder) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ private static void addMetaData(Map<String, Object> mappingsAsMap, String analyt
metadata.put(ANALYTICS, analyticsId);
}

@SuppressWarnings("unchecked")
private static <K, V> V getOrPutDefault(Map<K, Object> map, K key, Supplier<V> valueSupplier) {
V value = (V) map.get(key);
if (value == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ private void checkChecksumsMatch(DataFrameDataExtractor.Row row, RowResults resu
}

private IndexRequest createIndexRequest(RowResults result, SearchHit hit) {
Map<String, Object> source = new LinkedHashMap(hit.getSourceAsMap());
Map<String, Object> source = new LinkedHashMap<>(hit.getSourceAsMap());
source.putAll(result.getResults());
IndexRequest indexRequest = new IndexRequest(hit.getIndex());
indexRequest.id(hit.getId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public class RowResults implements ToXContentObject {
public static final ParseField CHECKSUM = new ParseField("checksum");
public static final ParseField RESULTS = new ParseField("results");

@SuppressWarnings("unchecked")
public static final ConstructingObjectParser<RowResults, Void> PARSER = new ConstructingObjectParser<>(TYPE.getPreferredName(),
a -> new RowResults((Integer) a[0], (Map<String, Object>) a[1]));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient
// We do not want to log anything due to a delete action
// The response or error will be returned to the client when called synchronously
// or it will be stored in the task result when called asynchronously
private static TaskListener nullTaskListener() {
return new TaskListener() {
private static <T> TaskListener<T> nullTaskListener() {
return new TaskListener<T>() {
@Override
public void onResponse(Task task, Object o) {}
public void onResponse(Task task, T o) {}

@Override
public void onFailure(Task task, Throwable e) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ private void givenJobs(List<Job> jobs, List<GetJobsStatsAction.Response.JobStats
jobListener.onResponse(
new QueryPage<>(jobs, jobs.size(), Job.RESULTS_FIELD));
return Void.TYPE;
}).when(jobManager).expandJobs(eq(MetaData.ALL), eq(true), any(ActionListener.class));
}).when(jobManager).expandJobs(eq(MetaData.ALL), eq(true), any());

doAnswer(invocationOnMock -> {
@SuppressWarnings("unchecked")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ clusterService, mock(Client.class), mock(Auditor.class), mock(PersistentTasksSer
jobConfigProvider, datafeedConfigProvider);
}

@SuppressWarnings("unchecked")
private void mockDatafeedConfigFindDatafeeds(Set<String> datafeedIds) {
doAnswer(invocation -> {
ActionListener<Set<String>> listener = (ActionListener<Set<String>>) invocation.getArguments()[1];
Expand All @@ -288,6 +289,7 @@ private void mockDatafeedConfigFindDatafeeds(Set<String> datafeedIds) {
}).when(datafeedConfigProvider).findDatafeedsForJobIds(any(), any(ActionListener.class));
}

@SuppressWarnings("unchecked")
private void mockJobConfigProviderExpandIds(Set<String> expandedIds) {
doAnswer(invocation -> {
ActionListener<Set<String>> listener = (ActionListener<Set<String>>) invocation.getArguments()[3];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class TransportFinalizeJobExecutionActionTests extends ESTestCase {
private Client client;

@Before
@SuppressWarnings("unchecked")
@SuppressWarnings({"unchecked", "rawtypes"})
private void setupMocks() {
ExecutorService executorService = mock(ExecutorService.class);
threadPool = mock(ThreadPool.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public class TransportPreviewDatafeedActionTests extends ESTestCase {
private Exception capturedFailure;

@Before
@SuppressWarnings("unchecked")
public void setUpTests() {
dataExtractor = mock(DataExtractor.class);
actionListener = mock(ActionListener.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public class DatafeedJobBuilderTests extends ESTestCase {
private DatafeedJobBuilder datafeedJobBuilder;

@Before
@SuppressWarnings("unchecked")
public void init() {
client = mock(Client.class);
ThreadPool threadPool = mock(ThreadPool.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -428,13 +428,13 @@ public static Job.Builder createDatafeedJob() {
return builder;
}

@SuppressWarnings({"rawtypes", "unchecked"})
private static DatafeedTask createDatafeedTask(String datafeedId, long startTime, Long endTime) {
DatafeedTask task = mock(DatafeedTask.class);
when(task.getDatafeedId()).thenReturn(datafeedId);
when(task.getDatafeedStartTime()).thenReturn(startTime);
when(task.getEndTime()).thenReturn(endTime);
doAnswer(invocationOnMock -> {
@SuppressWarnings("rawtypes")
ActionListener listener = (ActionListener) invocationOnMock.getArguments()[1];
listener.onResponse(mock(PersistentTask.class));
return null;
Expand All @@ -447,10 +447,10 @@ private Consumer<Exception> mockConsumer() {
return mock(Consumer.class);
}

@SuppressWarnings({"rawtypes", "unchecked"})
private DatafeedTask spyDatafeedTask(DatafeedTask task) {
task = spy(task);
doAnswer(invocationOnMock -> {
@SuppressWarnings("rawtypes")
ActionListener listener = (ActionListener) invocationOnMock.getArguments()[1];
listener.onResponse(mock(PersistentTask.class));
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ protected NamedXContentRegistry xContentRegistry() {
}

@Before
@SuppressWarnings({"rawtypes", "unchecked"})
public void setUpTests() {
client = mock(Client.class);
timingStatsReporter = mock(DatafeedTimingStatsReporter.class);
Expand All @@ -86,14 +87,12 @@ public void setUpTests() {
when(getRollupIndexResponse.getJobs()).thenReturn(new HashMap<>());

doAnswer(invocationMock -> {
@SuppressWarnings("raw_types")
ActionListener listener = (ActionListener) invocationMock.getArguments()[2];
listener.onResponse(fieldsCapabilities);
return null;
}).when(client).execute(same(FieldCapabilitiesAction.INSTANCE), any(), any());

doAnswer(invocationMock -> {
@SuppressWarnings("raw_types")
ActionListener listener = (ActionListener) invocationMock.getArguments()[2];
listener.onResponse(getRollupIndexResponse);
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ public Long getLastTimestamp() {
}

@Before
@SuppressWarnings("unchecked")
public void setUpTests() {
ThreadPool threadPool = mock(ThreadPool.class);
when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public class DataFrameDataExtractorTests extends ESTestCase {
private ActionFuture<ClearScrollResponse> clearScrollFuture;

@Before
@SuppressWarnings("unchecked")
public void setUpTests() {
ThreadPool threadPool = mock(ThreadPool.class);
when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public void testProcess_GivenSingleRowAndResult() throws IOException {
String dataDoc = "{\"f_1\": \"foo\", \"f_2\": 42.0}";
String[] dataValues = {"42.0"};
DataFrameDataExtractor.Row row = newRow(newHit(dataDoc), dataValues, 1);
givenDataFrameBatches(Arrays.asList(row));
givenDataFrameBatches(List.of(Arrays.asList(row)));

Map<String, Object> resultFields = new HashMap<>();
resultFields.put("a", "1");
Expand Down Expand Up @@ -97,7 +97,7 @@ public void testProcess_GivenFullResultsBatch() throws IOException {
IntStream.range(0, 1000).forEach(i -> firstBatch.add(newRow(newHit(dataDoc), dataValues, i)));
List<DataFrameDataExtractor.Row> secondBatch = new ArrayList<>(1);
secondBatch.add(newRow(newHit(dataDoc), dataValues, 1000));
givenDataFrameBatches(firstBatch, secondBatch);
givenDataFrameBatches(List.of(firstBatch, secondBatch));

Map<String, Object> resultFields = new HashMap<>();
resultFields.put("a", "1");
Expand All @@ -118,7 +118,7 @@ public void testProcess_GivenSingleRowAndResultWithMismatchingIdHash() throws IO
String dataDoc = "{\"f_1\": \"foo\", \"f_2\": 42.0}";
String[] dataValues = {"42.0"};
DataFrameDataExtractor.Row row = newRow(newHit(dataDoc), dataValues, 1);
givenDataFrameBatches(Arrays.asList(row));
givenDataFrameBatches(List.of(Arrays.asList(row)));

Map<String, Object> resultFields = new HashMap<>();
resultFields.put("a", "1");
Expand All @@ -136,7 +136,7 @@ public void testProcess_GivenSingleBatchWithSkippedRows() throws IOException {
String dataDoc = "{\"f_1\": \"foo\", \"f_2\": 42.0}";
String[] dataValues = {"42.0"};
DataFrameDataExtractor.Row normalRow = newRow(newHit(dataDoc), dataValues, 2);
givenDataFrameBatches(Arrays.asList(skippedRow, normalRow));
givenDataFrameBatches(List.of(Arrays.asList(skippedRow, normalRow)));

Map<String, Object> resultFields = new HashMap<>();
resultFields.put("a", "1");
Expand Down Expand Up @@ -166,7 +166,7 @@ public void testProcess_GivenTwoBatchesWhereFirstEndsWithSkippedRow() throws IOE
DataFrameDataExtractor.Row normalRow2 = newRow(newHit(dataDoc), dataValues, 2);
DataFrameDataExtractor.Row skippedRow = newRow(newHit("{}"), null, 3);
DataFrameDataExtractor.Row normalRow3 = newRow(newHit(dataDoc), dataValues, 4);
givenDataFrameBatches(Arrays.asList(normalRow1, normalRow2, skippedRow), Arrays.asList(normalRow3));
givenDataFrameBatches(List.of(Arrays.asList(normalRow1, normalRow2, skippedRow), Arrays.asList(normalRow3)));

Map<String, Object> resultFields = new HashMap<>();
resultFields.put("a", "1");
Expand Down Expand Up @@ -195,7 +195,7 @@ public void testProcess_GivenMoreResultsThanRows() throws IOException {
String dataDoc = "{\"f_1\": \"foo\", \"f_2\": 42.0}";
String[] dataValues = {"42.0"};
DataFrameDataExtractor.Row row = newRow(newHit(dataDoc), dataValues, 1);
givenDataFrameBatches(Arrays.asList(row));
givenDataFrameBatches(List.of(List.of(row)));

Map<String, Object> resultFields = new HashMap<>();
resultFields.put("a", "1");
Expand All @@ -214,7 +214,7 @@ public void testProcess_GivenNoResults_ShouldCancelAndConsumeExtractor() throws
String[] dataValues = {"42.0"};
DataFrameDataExtractor.Row row1 = newRow(newHit(dataDoc), dataValues, 1);
DataFrameDataExtractor.Row row2 = newRow(newHit(dataDoc), dataValues, 1);
givenDataFrameBatches(Arrays.asList(row1), Arrays.asList(row2));
givenDataFrameBatches(List.of(List.of(row1), List.of(row2)));

givenProcessResults(Collections.emptyList());

Expand All @@ -229,8 +229,8 @@ private void givenProcessResults(List<RowResults> results) {
}
}

private void givenDataFrameBatches(List<DataFrameDataExtractor.Row>... batches) throws IOException {
DelegateStubDataExtractor delegateStubDataExtractor = new DelegateStubDataExtractor(Arrays.asList(batches));
private void givenDataFrameBatches(List<List<DataFrameDataExtractor.Row>> batches) throws IOException {
DelegateStubDataExtractor delegateStubDataExtractor = new DelegateStubDataExtractor(batches);
when(dataExtractor.hasNext()).thenAnswer(a -> delegateStubDataExtractor.hasNext());
when(dataExtractor.next()).thenAnswer(a -> delegateStubDataExtractor.next());
}
Expand All @@ -254,6 +254,7 @@ private void givenClientHasNoFailures() {
ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
ThreadPool threadPool = mock(ThreadPool.class);
when(threadPool.getThreadContext()).thenReturn(threadContext);
@SuppressWarnings("unchecked")
ActionFuture<BulkResponse> responseFuture = mock(ActionFuture.class);
when(responseFuture.actionGet()).thenReturn(new BulkResponse(new BulkItemResponse[0], 0));
when(client.execute(same(BulkAction.INSTANCE), bulkRequestCaptor.capture())).thenReturn(responseFuture);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public void testCloseFailedJob() throws Exception {
PersistentTasksCustomMetaData tasks = state.getMetaData().custom(PersistentTasksCustomMetaData.TYPE);
assertEquals(1, tasks.taskMap().size());
// now just double check that the first job is still opened:
PersistentTasksCustomMetaData.PersistentTask task = tasks.getTask(MlTasks.jobTaskId("close-failed-job-1"));
PersistentTasksCustomMetaData.PersistentTask<?> task = tasks.getTask(MlTasks.jobTaskId("close-failed-job-1"));
assertEquals(JobState.OPENED, ((JobTaskState) task.getState()).getState());
}

Expand Down
Loading

0 comments on commit 23943ae

Please sign in to comment.