Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable XLint warnings for ML #44285

Merged
merged 4 commits into from
Jul 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use List.of everywhere like you did in line 217?
BTW, are you going to backport it to 7.x? This would require changing List.of to Arrays.asList everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this will be backported to 7.x which is why I didn't use List.of everywhere as I would have to undo the change in the backport.

I thought this partial change helped readability


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