Skip to content

Commit

Permalink
Replace Immutable(Map|Table).Builder.build() with buildOrThrow()
Browse files Browse the repository at this point in the history
See also: c0276ce, but this time with
Enforcer rules to prevent us from adding more.
  • Loading branch information
ksobolew authored and losipiuk committed Feb 21, 2022
1 parent a123f44 commit 509ac91
Show file tree
Hide file tree
Showing 23 changed files with 60 additions and 49 deletions.
11 changes: 11 additions & 0 deletions .mvn/modernizer/violations.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@
<comment>Prefer Math.toIntExact(long)</comment>
</violation>

<violation>
<name>com/google/common/collect/ImmutableMap$Builder.build:()Lcom/google/common/collect/ImmutableMap;</name>
<version>1.8</version>
<comment>Use buildOrThrow() instead, as it makes it clear that it will throw on duplicated values</comment>
</violation>
<violation>
<name>com/google/common/collect/ImmutableTable$Builder.build:()Lcom/google/common/collect/ImmutableTable;</name>
<version>1.8</version>
<comment>Use buildOrThrow() instead, as it makes it clear that it will throw on duplicated values</comment>
</violation>

<violation>
<name>com/google/common/cache/CacheBuilder.build:()Lcom/google/common/cache/Cache;</name>
<version>1.8</version>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public <S, T> Builder add(String sourceRawType, Class<S> sourceClass, Class<T> t

public TypeConversions build()
{
return new TypeConversions(conversions.build());
return new TypeConversions(conversions.buildOrThrow());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1780,7 +1780,7 @@ public static FaultTolerantDistributedStagesScheduler create(
taskLifecycleListener,
exchange,
bucketToPartitionCache.apply(fragment.getPartitioningScheme().getPartitioning().getHandle()).getBucketToPartitionMap(),
sourceExchanges.build(),
sourceExchanges.buildOrThrow(),
inputBucketToPartition.getBucketToPartitionMap(),
inputBucketToPartition.getBucketNodeMap(),
retryAttempts);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ private static Map<PlanFragmentId, PlanNodeId> getSourceFragmentToRemoteSourceNo
result.put(sourceFragmentId, node.getId());
}
}
return result.build();
return result.buildOrThrow();
}

private static ListMultimap<PlanNodeId, ExchangeSourceHandle> getInputsForRemoteSources(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public void testPollPagesQueryLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(0, 0), new RuntimeException("error"))
.build(),
.buildOrThrow(),
DataSize.of(5, KILOBYTE),
0,
ImmutableList.of(
Expand Down Expand Up @@ -246,7 +246,7 @@ public void testPollPagesQueryLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(0, 0), new RuntimeException("error"))
.build(),
.buildOrThrow(),
DataSize.of(5, KILOBYTE),
2,
ImmutableList.of(
Expand All @@ -262,7 +262,7 @@ public void testPollPagesQueryLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(0, 0), new RuntimeException("error"))
.build(),
.buildOrThrow(),
DataSize.of(5, KILOBYTE),
2,
ImmutableList.of(
Expand All @@ -279,7 +279,7 @@ public void testPollPagesQueryLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(2, 0), new RuntimeException("error"))
.build(),
.buildOrThrow(),
DataSize.of(5, KILOBYTE),
0,
ImmutableList.of(
Expand All @@ -295,7 +295,7 @@ public void testPollPagesQueryLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(2, 0), new RuntimeException("error"))
.build(),
.buildOrThrow(),
DataSize.of(4, KILOBYTE),
3,
ImmutableList.of(
Expand All @@ -313,7 +313,7 @@ public void testPollPagesQueryLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(2, 2), error)
.build(),
.buildOrThrow(),
DataSize.of(4, KILOBYTE),
0,
error);
Expand All @@ -328,7 +328,7 @@ public void testPollPagesQueryLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(0, 1), error)
.build(),
.buildOrThrow(),
DataSize.of(4, KILOBYTE),
0,
error);
Expand All @@ -343,7 +343,7 @@ public void testPollPagesQueryLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(2, 2), error)
.build(),
.buildOrThrow(),
DataSize.of(3, KILOBYTE),
3,
error);
Expand All @@ -358,7 +358,7 @@ public void testPollPagesQueryLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(0, 1), error)
.build(),
.buildOrThrow(),
DataSize.of(3, KILOBYTE),
3,
error);
Expand Down Expand Up @@ -464,7 +464,7 @@ public void testPollPagesTaskLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(1, 0), new RuntimeException("error"))
.build(),
.buildOrThrow(),
DataSize.of(10, KILOBYTE),
0,
ImmutableList.of(
Expand All @@ -481,7 +481,7 @@ public void testPollPagesTaskLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(1, 0), new RuntimeException("error"))
.build(),
.buildOrThrow(),
DataSize.of(2, KILOBYTE),
3,
ImmutableList.of(
Expand All @@ -500,7 +500,7 @@ public void testPollPagesTaskLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(2, 2), error)
.build(),
.buildOrThrow(),
DataSize.of(5, KILOBYTE),
0,
error);
Expand All @@ -514,7 +514,7 @@ public void testPollPagesTaskLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(0, 1), error)
.build(),
.buildOrThrow(),
DataSize.of(5, KILOBYTE),
0,
error);
Expand All @@ -529,7 +529,7 @@ public void testPollPagesTaskLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(2, 2), error)
.build(),
.buildOrThrow(),
DataSize.of(2, KILOBYTE),
3,
error);
Expand All @@ -543,7 +543,7 @@ public void testPollPagesTaskLevelRetry()
.build(),
ImmutableMap.<TaskId, RuntimeException>builder()
.put(createTaskId(0, 1), error)
.build(),
.buildOrThrow(),
DataSize.of(1, KILOBYTE),
2,
error);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ public void testOAuth2Groups(Optional<Set<String>> groups)
.put("http-server.authentication.type", "oauth2")
.putAll(getOAuth2Properties(tokenServer))
.put("http-server.authentication.oauth2.groups-field", GROUPS_CLAIM)
.build())
.buildOrThrow())
.setAdditionalModule(oauth2Module(tokenServer))
.build()) {
server.getInstance(Key.get(AccessControlManager.class)).addSystemAccessControl(TestSystemAccessControl.NO_IMPERSONATION);
Expand Down Expand Up @@ -813,7 +813,7 @@ private static Map<String, String> getOAuth2Properties(TokenServer tokenServer)
.put("http-server.authentication.oauth2.token-url", tokenServer.getIssuer())
.put("http-server.authentication.oauth2.client-id", tokenServer.getClientId())
.put("http-server.authentication.oauth2.client-secret", tokenServer.getClientSecret())
.build();
.buildOrThrow();
}

private static String getOauthToken(OkHttpClient client, String url)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public void init()
.withName("mockmissingcolumns")
.withGetViews((s, prefix) -> ImmutableMap.<SchemaTableName, ConnectorViewDefinition>builder()
.put(new SchemaTableName("default", "nation_view"), view)
.build())
.buildOrThrow())
.withGetColumns(schemaTableName -> {
if (schemaTableName.equals(new SchemaTableName("tiny", "nation_with_optional_column"))) {
return TPCH_NATION_WITH_OPTIONAL_COLUMN;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ public Map<String, Object> getTableProperties(ConnectorSession session, JdbcTabl
properties.put(SAMPLE_BY_PROPERTY, samplingKey);
}
}
return properties.build();
return properties.buildOrThrow();
}
}
catch (SQLException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,8 +430,8 @@ public void testArrayFields()
.add("hello")
.add("world")
.build())
.build())
.build())
.buildOrThrow())
.buildOrThrow())
.put("c", ImmutableMap.<String, Object>builder()
.put("d", "foo")
.put("e", "bar")
Expand All @@ -442,16 +442,16 @@ public void testArrayFields()
.add(20)
.build())
.put("h", 100)
.build())
.buildOrThrow())
.add(ImmutableMap.<String, Object>builder()
.put("g", ImmutableList.<Integer>builder()
.add(30)
.add(40)
.build())
.put("h", 200)
.build())
.buildOrThrow())
.build())
.build())
.buildOrThrow())
.put("j", ImmutableList.<Long>builder()
.add(50L)
.add(60L)
Expand Down Expand Up @@ -571,7 +571,7 @@ public void testAsRawJson()
.add(345)
.build())
.build())
.build())
.buildOrThrow())
.put("es_array_object", ImmutableMap.<String, Object>builder()
.put("array_of_string_arrays", ImmutableList.<List<String>>builder()
.add(ImmutableList.<String>builder()
Expand All @@ -586,7 +586,7 @@ public void testAsRawJson()
.add(345)
.build())
.build())
.build())
.buildOrThrow())
.put("es_raw_object", ImmutableMap.<String, Object>builder()
.put("array_of_string_arrays", ImmutableList.<List<String>>builder()
.add(ImmutableList.<String>builder()
Expand All @@ -601,7 +601,7 @@ public void testAsRawJson()
.add(345)
.build())
.build())
.build())
.buildOrThrow())
.put("array_of_string_arrays", ImmutableList.<List<String>>builder()
.add(ImmutableList.<String>builder()
.add("abc")
Expand All @@ -622,15 +622,15 @@ public void testAsRawJson()
.put("es_object", ImmutableMap.<String, Object>builder()
.put("array_of_string_arrays", "Join the Trino Slack: https://trino.io/slack.html")
.put("arrayOfIntArrays", 867)
.build())
.buildOrThrow())
.put("es_array_object", ImmutableMap.<String, Object>builder()
.put("array_of_string_arrays", "If you like Presto, you'll love Trino: https://trino.io/slack.html")
.put("arrayOfIntArrays", 321)
.build())
.buildOrThrow())
.put("es_raw_object", ImmutableMap.<String, Object>builder()
.put("array_of_string_arrays", "The founders and core contributors of Presto, and are now working on Trino: https://trino.io/blog/2020/12/27/announcing-trino.html")
.put("arrayOfIntArrays", 654)
.build())
.buildOrThrow())
.put("array_of_string_arrays", "Check out the bi-weekly Trino Community Broadcast https://trino.io/broadcast/")
.put("array_of_long_arrays", 5309L)
.put("order_field", 2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ private Map<Integer, FileStatus> getCommittedPartitions(URI committedAttemptPath
int partitionId = Integer.parseInt(matcher.group(1));
result.put(partitionId, partitionFile);
}
return result.build();
return result.buildOrThrow();
}
catch (IOException e) {
throw new UncheckedIOException(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void testExplicitPropertyMappings()
.put("exchange.base-directory", "s3n://exchange-spooling-test/")
.put("exchange.encryption-enabled", "true")
.put("exchange.sink-buffer-pool-min-size", "10")
.build();
.buildOrThrow();

FileSystemExchangeConfig expected = new FileSystemExchangeConfig()
.setBaseDirectory("s3n://exchange-spooling-test/")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public MinioStorage(String bucketName)
.withEnvVars(ImmutableMap.<String, String>builder()
.put("MINIO_ACCESS_KEY", ACCESS_KEY)
.put("MINIO_SECRET_KEY", SECRET_KEY)
.build())
.buildOrThrow())
.build();
}

Expand Down Expand Up @@ -92,6 +92,6 @@ public static Map<String, String> getExchangeManagerProperties(MinioStorage mini
.put("exchange.s3.region", "us-east-1")
// TODO: enable exchange encryption after https is supported for Trino MinIO
.put("exchange.s3.endpoint", "http://" + minioStorage.getMinio().getMinioApiEndpoint())
.build();
.buildOrThrow();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void testExplicitPropertyMappings()
.put("exchange.s3.endpoint", "https://s3.us-east-1.amazonaws.com")
.put("exchange.s3.max-error-retries", "8")
.put("exchange.s3.upload.part-size", "10MB")
.build();
.buildOrThrow();

ExchangeS3Config expected = new ExchangeS3Config()
.setS3AwsAccessKey("access")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ protected QueryRunner createQueryRunner(
// When streaming upload is enabled insert into a table with high number of buckets / partitions may cause
// the tests to run out of memory as the buffer space is eagerly allocated for each output file.
.put("hive.s3.streaming.enabled", "false")
.build())
.buildOrThrow())
.setExchangeManagerProperties(getExchangeManagerProperties(minioStorage))
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ protected QueryRunner createQueryRunner(
.putAll(configProperties)
// currently not supported for fault tolerant execution mode
.put("enable-dynamic-filtering", "false")
.build())
.buildOrThrow())
.setCoordinatorProperties(coordinatorProperties)
.setHiveProperties(ImmutableMap.<String, String>builder()
// Streaming upload allocates non trivial amount of memory for buffering (16MB per output file by default).
// When streaming upload is enabled insert into a table with high number of buckets / partitions may cause
// the tests to run out of memory as the buffer space is eagerly allocated for each output file.
.put("hive.s3.streaming.enabled", "false")
.build())
.buildOrThrow())
.setExchangeManagerProperties(getExchangeManagerProperties(minioStorage))
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void testExplicitPropertyMappings()
Map<String, String> properties = ImmutableMap.<String, String>builder()
.put("hive.user-metastore-cache-ttl", "2h")
.put("hive.user-metastore-cache-maximum-size", "5")
.build();
.buildOrThrow();

ImpersonationCachingConfig expected = new ImpersonationCachingConfig()
.setUserMetastoreCacheTtl(new Duration(2, TimeUnit.HOURS))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public DistributedQueryRunner build()
Map<String, String> icebergProperties = new HashMap<>();
icebergProperties.put("iceberg.catalog.type", "TESTING_FILE_METASTORE");
icebergProperties.put("hive.metastore.catalog.dir", dataDir.toString());
icebergProperties.putAll(this.icebergProperties.build());
icebergProperties.putAll(this.icebergProperties.buildOrThrow());

queryRunner.createCatalog(ICEBERG_CATALOG, "iceberg", icebergProperties);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ protected QueryRunner createQueryRunner(
.putAll(configProperties)
// currently not supported for fault tolerant execution mode
.put("enable-dynamic-filtering", "false")
.build())
.buildOrThrow())
.setExchangeManagerProperties(getExchangeManagerProperties(minioStorage))
.build();
}
Expand Down
Loading

0 comments on commit 509ac91

Please sign in to comment.