From 8dbe77122cc7f5bdb1e4101e954c784d18152ac7 Mon Sep 17 00:00:00 2001 From: Christopher Lambert Date: Thu, 11 May 2023 13:01:49 +0200 Subject: [PATCH 1/3] increase severity of multiple errorprone checks note that we fix DifferentNameButSame with a minimal diff, but not make it uniform across all files --- codestyle/errorprone-rules.properties | 46 +++++++++++-------- .../tests/AbstractCompatibilityTests.java | 4 +- .../compatibility/tests/ITUpgradePath.java | 8 ++-- .../gc/iceberg/mocks/MockManifestEntry.java | 3 +- .../gc/iceberg/mocks/MockManifestFile.java | 5 +- .../spark/extensions/SparkSqlTestBase.java | 7 ++- .../jaxrs/tests/BaseTestNessieApi.java | 6 +-- .../ReferenceTypeParamConverterProvider.java | 5 +- .../projectnessie/services/impl/RefUtil.java | 2 +- .../services/impl/TreeApiImpl.java | 6 +-- .../services/spi/TreeService.java | 3 +- .../services/impl/TestRefUtil.java | 2 +- .../services/impl/AbstractTestCommitLog.java | 13 +++--- .../services/impl/AbstractTestReferences.java | 3 +- .../services/impl/BaseTestServiceImpl.java | 7 +-- .../server/store/TestStoreWorker.java | 4 +- .../adapter/spi/AbstractDatabaseAdapter.java | 4 +- .../adapter/serialize/ProtoSerialization.java | 5 +- .../adapter/serialize/TestSerialization.java | 4 +- .../persist/tx/TxDatabaseAdapter.java | 3 +- .../versioned/TestMetricsVersionStore.java | 2 +- .../common/objtypes/CommitHeadersImpl.java | 4 +- .../versionstore/BaseCommitHelper.java | 3 +- 23 files changed, 70 insertions(+), 79 deletions(-) diff --git a/codestyle/errorprone-rules.properties b/codestyle/errorprone-rules.properties index 5df12c664a0..914219a7b37 100644 --- a/codestyle/errorprone-rules.properties +++ b/codestyle/errorprone-rules.properties @@ -966,10 +966,10 @@ MissingOverride=ERROR #MixedMutabilityReturnType=WARN # This method returns both mutable and immutable collections or maps from different paths. This may be confusing for users of the method. -#ModifiedButNotUsed=WARN +ModifiedButNotUsed=ERROR # A collection or proto builder was created, but its values were never accessed. -#MockNotUsedInProduction=WARN +MockNotUsedInProduction=ERROR # This mock is instantiated and configured, but is never passed to production code. It should be # either removed or used. @@ -1000,16 +1000,19 @@ MissingOverride=ERROR #NestedInstanceOfConditions=WARN # Nested instanceOf conditions of disjoint types create blocks of code that never execute -#NonAtomicVolatileUpdate=WARN +NonAtomicVolatileUpdate=ERROR # This update of a volatile variable is non-atomic -#NonCanonicalType=WARN +NonCanonicalType=ERROR # This type is referred to by a non-canonical name, which may be misleading. #NonOverridingEquals=WARN # equals method doesn't override Object.equals -#NullOptional=WARN +NotJavadoc=ERROR +# Avoid using /** for comments which aren't actually Javadoc. + +NullOptional=ERROR # Passing a literal null to an Optional parameter is almost certainly a mistake. Did you mean to provide an empty Optional? #NullableConstructor=WARN @@ -1222,10 +1225,10 @@ UnnecessaryLambda=ERROR UnusedMethod=ERROR # Unused. -#UnusedNestedClass=WARN +UnusedNestedClass=ERROR # This nested class is unused, and can be removed. -#UnusedTypeParameter=WARN +UnusedTypeParameter=ERROR # This type parameter is unused and can be removed. #UnusedVariable=WARN @@ -1349,10 +1352,10 @@ UseCorrectAssertInTests=ERROR #CheckedExceptionNotThrown=WARN # This method cannot throw a checked exception that it claims to. This may cause consumers of the API to incorrectly attempt to handle, or propagate, this exception. -ConstantPatternCompile=WARN +ConstantPatternCompile=ERROR # Variables initialized with Pattern#compile calls on constants can be constants -#DifferentNameButSame=WARN +DifferentNameButSame=ERROR # This type is referred to in different ways within this file, which may be confusing. #EqualsBrokenForNull=WARN @@ -1415,7 +1418,7 @@ ConstantPatternCompile=WARN #PreferredInterfaceType=WARN # This type can be more specific. -PrimitiveArrayPassedToVarargsMethod=WARN +PrimitiveArrayPassedToVarargsMethod=ERROR # Passing a primitive array to a varargs method is usually wrong #QualifierWithTypeUse=WARN @@ -1427,10 +1430,13 @@ RedundantOverride=ERROR RedundantThrows=ERROR # Thrown exception is a subtype of another +StringCaseLocaleUsage=ERROR +# Specify a `Locale` when calling `String#to{Lower,Upper}Case`. (Note: there are multiple suggested fixes; the third may be most appropriate if you're dealing with ASCII Strings.) + StronglyTypeByteString=WARN # This primitive byte array is only used to construct ByteStrings. It would be clearer to strongly type the field instead. -StronglyTypeTime=WARN +StronglyTypeTime=ERROR # This primitive integral type is only used to construct time types. It would be clearer to strongly type the field instead. #SuppressWarningsWithoutExplanation=WARN @@ -1442,7 +1448,7 @@ StronglyTypeTime=WARN #SystemOut=WARN # Printing to standard output should only be used for debugging, not in production code -#TestExceptionChecker=WARN +TestExceptionChecker=ERROR # Using @Test(expected=…) is discouraged, since the test will pass if any statement in the test method throws the expected exception #ThrowSpecificExceptions=WARN @@ -1454,7 +1460,7 @@ StronglyTypeTime=WARN #TooManyParameters=WARN # A large number of parameters on public APIs should be avoided. -#TransientMisuse=WARN +TransientMisuse=ERROR # Static fields are implicitly transient, so the explicit modifier is unnecessary #TryWithResourcesVariable=WARN @@ -1484,7 +1490,7 @@ StronglyTypeTime=WARN #UnusedException=WARN # This catch block catches an exception and re-throws another, but swallows the caught exception rather than setting it as a cause. This can make debugging harder. -#UrlInSee=WARN +UrlInSee=ERROR # URLs should not be used in @see tags; they are designed for Java elements which could be used with @link. #UsingJsr305CheckReturnValue=WARN @@ -1525,13 +1531,13 @@ StronglyTypeTime=WARN #FieldCanBeLocal=WARN # This field can be replaced with a local variable in the methods that use it. -#FieldCanBeStatic=WARN +FieldCanBeStatic=ERROR # A final field initialized at compile-time with an instance of an immutable type can be static. #FieldMissingNullable=WARN # Field is assigned (or compared against) a definitely null value but is not annotated @Nullable -#ForEachIterable=WARN +ForEachIterable=ERROR # This loop can be replaced with an enhanced for loop. #ImmutableMemberCollection=WARN @@ -1555,7 +1561,7 @@ StronglyTypeTime=WARN #MissingBraces=WARN # The Google Java Style Guide requires braces to be used with if, else, for, do and while statements, even when the body is empty or contains only a single statement. -#MixedArrayDimensions=WARN +MixedArrayDimensions=ERROR # C-style array declarations should not be used #MultiVariableDeclaration=WARN @@ -1564,7 +1570,7 @@ StronglyTypeTime=WARN #MultipleTopLevelClasses=WARN # Source files should not contain multiple top-level class declarations -#PackageLocation=WARN +PackageLocation=ERROR # Package names should match the directory they are declared in #ParameterComment=WARN @@ -1603,7 +1609,7 @@ StronglyTypeTime=WARN #ThrowsUncheckedException=WARN # Unchecked exceptions do not need to be declared in the method signature. -TryFailRefactoring=WARN +TryFailRefactoring=ERROR # Prefer assertThrows to try/fail #TypeParameterNaming=WARN @@ -1627,7 +1633,7 @@ UnnecessaryBoxedVariable=ERROR #UnnecessaryStaticImport=WARN # Using static imports for types is unnecessary -UseEnumSwitch=WARN +UseEnumSwitch=ERROR # Prefer using a switch instead of a chained if-else for enums #VoidMissingNullable=WARN diff --git a/compatibility/compatibility-tests/src/intTest/java/org/projectnessie/tools/compatibility/tests/AbstractCompatibilityTests.java b/compatibility/compatibility-tests/src/intTest/java/org/projectnessie/tools/compatibility/tests/AbstractCompatibilityTests.java index a07d2db5b8d..39c2d5a94fa 100644 --- a/compatibility/compatibility-tests/src/intTest/java/org/projectnessie/tools/compatibility/tests/AbstractCompatibilityTests.java +++ b/compatibility/compatibility-tests/src/intTest/java/org/projectnessie/tools/compatibility/tests/AbstractCompatibilityTests.java @@ -41,7 +41,6 @@ import org.projectnessie.model.Content; import org.projectnessie.model.ContentKey; import org.projectnessie.model.IcebergTable; -import org.projectnessie.model.LogResponse; import org.projectnessie.model.LogResponse.LogEntry; import org.projectnessie.model.MergeBehavior; import org.projectnessie.model.MergeResponse; @@ -88,8 +87,7 @@ Stream allReferences() throws NessieNotFoundException { } @SuppressWarnings("deprecation") - Stream commitLog( - Function configurer) + Stream commitLog(Function configurer) throws NessieNotFoundException { if (getClientVersion().isGreaterThanOrEqual(Version.CLIENT_RESULTS_NATIVE_STREAM)) { return configurer.apply(api.getCommitLog()).stream(); diff --git a/compatibility/compatibility-tests/src/intTest/java/org/projectnessie/tools/compatibility/tests/ITUpgradePath.java b/compatibility/compatibility-tests/src/intTest/java/org/projectnessie/tools/compatibility/tests/ITUpgradePath.java index 9df2bb6684e..58b92e90ede 100644 --- a/compatibility/compatibility-tests/src/intTest/java/org/projectnessie/tools/compatibility/tests/ITUpgradePath.java +++ b/compatibility/compatibility-tests/src/intTest/java/org/projectnessie/tools/compatibility/tests/ITUpgradePath.java @@ -61,10 +61,8 @@ import org.projectnessie.model.EntriesResponse; import org.projectnessie.model.IcebergTable; import org.projectnessie.model.LogResponse; -import org.projectnessie.model.LogResponse.LogEntry; import org.projectnessie.model.Operation.Delete; import org.projectnessie.model.Operation.Put; -import org.projectnessie.model.RefLogResponse; import org.projectnessie.model.RefLogResponse.RefLogResponseEntry; import org.projectnessie.model.Reference; import org.projectnessie.tools.compatibility.api.NessieAPI; @@ -132,8 +130,8 @@ Stream commitLog( } @SuppressWarnings("deprecation") - Stream refLog( - Function configurer) throws NessieNotFoundException { + Stream refLog(Function configurer) + throws NessieNotFoundException { if (version.isGreaterThan(Version.parseVersion("0.30.0"))) { return configurer.apply(api.getRefLog()).stream(); } else { @@ -221,7 +219,7 @@ void commitLog() throws NessieNotFoundException { String commitMessage = "hello world " + versionFromRef; assertThat(commitLog) .hasSize(1) - .map(LogEntry::getCommitMeta) + .map(LogResponse.LogEntry::getCommitMeta) .map(CommitMeta::getMessage) .containsExactly(commitMessage); }) diff --git a/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestEntry.java b/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestEntry.java index cb4f1d23dab..713c7b4ed13 100644 --- a/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestEntry.java +++ b/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestEntry.java @@ -24,7 +24,6 @@ import org.apache.iceberg.BridgeToIceberg; import org.apache.iceberg.avro.AvroSchemaUtil; import org.apache.iceberg.types.Types; -import org.apache.iceberg.types.Types.StructType; import org.immutables.value.Value; @Value.Immutable @@ -51,7 +50,7 @@ public Status status() { public abstract String filePath(); - public abstract StructType partitionType(); + public abstract Types.StructType partitionType(); // ids for data-file columns are assigned from 1000 static final Types.NestedField STATUS = required(0, "status", Types.IntegerType.get()); diff --git a/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestFile.java b/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestFile.java index 43a2663f9fb..b60f49cc964 100644 --- a/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestFile.java +++ b/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestFile.java @@ -39,7 +39,6 @@ import org.apache.iceberg.io.FileAppender; import org.apache.iceberg.io.OutputFile; import org.apache.iceberg.types.Types; -import org.apache.iceberg.types.Types.StructType; import org.immutables.value.Value; @Value.Immutable @@ -143,7 +142,7 @@ public List manifestEntries() { MockPartitionSpec mockPartitionSpec = mockTableMeta.partitionSpec(0); org.apache.iceberg.Schema icebergSchema = mockTableMeta.schema(0).toSchema(); PartitionSpec partitionSpec = mockPartitionSpec.toPartitionSpec(icebergSchema); - StructType partitionType = partitionSpec.partitionType(); + Types.StructType partitionType = partitionSpec.partitionType(); return IntStream.range(0, numEntries()) .mapToObj( @@ -164,7 +163,7 @@ public void write(OutputFile output) { MockPartitionSpec mockPartitionSpec = mockTableMeta.partitionSpec(0); org.apache.iceberg.Schema icebergSchema = mockTableMeta.schema(0).toSchema(); PartitionSpec partitionSpec = mockPartitionSpec.toPartitionSpec(icebergSchema); - StructType partitionType = partitionSpec.partitionType(); + Types.StructType partitionType = partitionSpec.partitionType(); try { String partitionSpecJson = PartitionSpecParser.toJson(partitionSpec); diff --git a/integrations/spark-extensions-basetests/src/main/java/org/projectnessie/spark/extensions/SparkSqlTestBase.java b/integrations/spark-extensions-basetests/src/main/java/org/projectnessie/spark/extensions/SparkSqlTestBase.java index f182163fe55..7893c1ad435 100644 --- a/integrations/spark-extensions-basetests/src/main/java/org/projectnessie/spark/extensions/SparkSqlTestBase.java +++ b/integrations/spark-extensions-basetests/src/main/java/org/projectnessie/spark/extensions/SparkSqlTestBase.java @@ -51,8 +51,6 @@ import org.projectnessie.model.ImmutableOperations; import org.projectnessie.model.Namespace; import org.projectnessie.model.Operation; -import org.projectnessie.model.Operation.Delete; -import org.projectnessie.model.Operation.Put; import org.projectnessie.model.Operations; import org.projectnessie.model.Reference; import org.projectnessie.model.Tag; @@ -107,13 +105,14 @@ protected void setupSparkAndApi(TestInfo testInfo) api.commitMultipleOperations() .branch(initialDefaultBranch) .commitMeta(CommitMeta.fromMessage("INFRA: initial commit")) - .operation(Put.of(ContentKey.of("dummy"), IcebergTable.of("foo", 1, 2, 3, 4))) + .operation( + Operation.Put.of(ContentKey.of("dummy"), IcebergTable.of("foo", 1, 2, 3, 4))) .commit(); initialDefaultBranch = api.commitMultipleOperations() .branch(initialDefaultBranch) .commitMeta(CommitMeta.fromMessage("INFRA: common ancestor")) - .operation(Delete.of(ContentKey.of("dummy"))) + .operation(Operation.Delete.of(ContentKey.of("dummy"))) .commit(); first = false; } diff --git a/servers/jax-rs-tests/src/main/java/org/projectnessie/jaxrs/tests/BaseTestNessieApi.java b/servers/jax-rs-tests/src/main/java/org/projectnessie/jaxrs/tests/BaseTestNessieApi.java index b9ed677dfca..030e5386cfb 100644 --- a/servers/jax-rs-tests/src/main/java/org/projectnessie/jaxrs/tests/BaseTestNessieApi.java +++ b/servers/jax-rs-tests/src/main/java/org/projectnessie/jaxrs/tests/BaseTestNessieApi.java @@ -83,7 +83,6 @@ import org.projectnessie.model.DiffResponse; import org.projectnessie.model.DiffResponse.DiffEntry; import org.projectnessie.model.EntriesResponse; -import org.projectnessie.model.EntriesResponse.Entry; import org.projectnessie.model.GetMultipleContentsResponse; import org.projectnessie.model.GetNamespacesResponse; import org.projectnessie.model.IcebergTable; @@ -1371,7 +1370,7 @@ public void commitLogForNamelessReference() throws BaseNessieClientServerExcepti branch = prepCommit(branch, "c-" + i, dummyPut("c", Integer.toString(i))).commit(); } } - List log = + List log = api().getCommitLog().hashOnRef(branch.getHash()).stream().collect(Collectors.toList()); // Verifying size is sufficient to make sure the right log was retrieved assertThat(log).hasSize(5); @@ -1440,7 +1439,8 @@ public void fetchEntriesByNamelessReference() throws BaseNessieClientServerExcep .operation(Put.of(b, tb)) .commitMeta(CommitMeta.fromMessage("commit 1")) .commit(); - List entries = api().getEntries().hashOnRef(branch.getHash()).get().getEntries(); + List entries = + api().getEntries().hashOnRef(branch.getHash()).get().getEntries(); soft.assertThat(entries) .map(e -> immutableEntry(e.getName(), e.getType())) .containsExactlyInAnyOrder( diff --git a/servers/rest-services/src/main/java/org/projectnessie/services/restjavax/ReferenceTypeParamConverterProvider.java b/servers/rest-services/src/main/java/org/projectnessie/services/restjavax/ReferenceTypeParamConverterProvider.java index 12a556117ad..891936b2fad 100644 --- a/servers/rest-services/src/main/java/org/projectnessie/services/restjavax/ReferenceTypeParamConverterProvider.java +++ b/servers/rest-services/src/main/java/org/projectnessie/services/restjavax/ReferenceTypeParamConverterProvider.java @@ -21,12 +21,11 @@ import javax.ws.rs.ext.ParamConverter; import javax.ws.rs.ext.ParamConverterProvider; import javax.ws.rs.ext.Provider; -import org.projectnessie.model.Reference; import org.projectnessie.model.Reference.ReferenceType; /** * Provider for {@link ReferenceTypeParamConverter}, to convert between lower-case representations - * of {@link Reference.ReferenceType} in REST paths and upper-case in the Java enum.. + * of {@link ReferenceType} in REST paths and upper-case in the Java enum.. */ @Provider public class ReferenceTypeParamConverterProvider implements ParamConverterProvider { @@ -37,7 +36,7 @@ public class ReferenceTypeParamConverterProvider implements ParamConverterProvid @Override public ParamConverter getConverter( Class rawType, Type genericType, Annotation[] annotations) { - if (rawType == Reference.ReferenceType.class) { + if (rawType == ReferenceType.class) { @SuppressWarnings("unchecked") ParamConverter r = (ParamConverter) REFERENCE_TYPE_PARAM_CONVERTER; return r; diff --git a/servers/services/src/main/java/org/projectnessie/services/impl/RefUtil.java b/servers/services/src/main/java/org/projectnessie/services/impl/RefUtil.java index c690eed3290..bb01269154d 100644 --- a/servers/services/src/main/java/org/projectnessie/services/impl/RefUtil.java +++ b/servers/services/src/main/java/org/projectnessie/services/impl/RefUtil.java @@ -47,7 +47,7 @@ public static NamedRef toNamedRef(@Nonnull @jakarta.annotation.Nonnull Reference } public static NamedRef toNamedRef( - @Nonnull @jakarta.annotation.Nonnull Reference.ReferenceType referenceType, + @Nonnull @jakarta.annotation.Nonnull ReferenceType referenceType, @Nonnull @jakarta.annotation.Nonnull String referenceName) { Objects.requireNonNull(referenceType, "referenceType must not be null"); switch (referenceType) { diff --git a/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java b/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java index 461ed222f95..a5ea55968f4 100644 --- a/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java +++ b/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java @@ -76,7 +76,6 @@ import org.projectnessie.model.ContentKey; import org.projectnessie.model.Detached; import org.projectnessie.model.EntriesResponse; -import org.projectnessie.model.EntriesResponse.Entry; import org.projectnessie.model.FetchOption; import org.projectnessie.model.ImmutableCommitMeta; import org.projectnessie.model.ImmutableCommitResponse; @@ -92,7 +91,6 @@ import org.projectnessie.model.Operation; import org.projectnessie.model.Operations; import org.projectnessie.model.Reference; -import org.projectnessie.model.Reference.ReferenceType; import org.projectnessie.model.ReferenceMetadata; import org.projectnessie.model.Tag; import org.projectnessie.model.Validation; @@ -327,7 +325,7 @@ public Reference assignReference( @Override public Reference deleteReference( - ReferenceType referenceType, String referenceName, String expectedHash) + Reference.ReferenceType referenceType, String referenceName, String expectedHash) throws NessieConflictException, NessieNotFoundException { try { ReferenceInfo resolved = @@ -780,7 +778,7 @@ public R getEntries( String filter, String pagingToken, boolean withContent, - PagedResponseHandler pagedResponseHandler, + PagedResponseHandler pagedResponseHandler, Consumer> effectiveReference) throws NessieNotFoundException { WithHash refWithHash = namedRefWithHashOrThrow(namedRef, hashOnRef); diff --git a/servers/services/src/main/java/org/projectnessie/services/spi/TreeService.java b/servers/services/src/main/java/org/projectnessie/services/spi/TreeService.java index b48ac95d064..9048c4125da 100644 --- a/servers/services/src/main/java/org/projectnessie/services/spi/TreeService.java +++ b/servers/services/src/main/java/org/projectnessie/services/spi/TreeService.java @@ -36,7 +36,6 @@ import org.projectnessie.model.MergeResponse; import org.projectnessie.model.Operations; import org.projectnessie.model.Reference; -import org.projectnessie.model.Reference.ReferenceType; import org.projectnessie.model.Validation; import org.projectnessie.versioned.NamedRef; import org.projectnessie.versioned.WithHash; @@ -121,7 +120,7 @@ Reference assignReference( throws NessieNotFoundException, NessieConflictException; Reference deleteReference( - ReferenceType referenceType, + Reference.ReferenceType referenceType, @Valid @jakarta.validation.Valid @NotNull diff --git a/servers/services/src/test/java/org/projectnessie/services/impl/TestRefUtil.java b/servers/services/src/test/java/org/projectnessie/services/impl/TestRefUtil.java index d7d41d0e3f4..4494bf68145 100644 --- a/servers/services/src/test/java/org/projectnessie/services/impl/TestRefUtil.java +++ b/servers/services/src/test/java/org/projectnessie/services/impl/TestRefUtil.java @@ -73,7 +73,7 @@ public ReferenceMetadata getMetadata() { } @Override - public ReferenceType getType() { + public Reference.ReferenceType getType() { return null; } })) diff --git a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestCommitLog.java b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestCommitLog.java index dd821d473e8..6e361fe77f2 100644 --- a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestCommitLog.java +++ b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestCommitLog.java @@ -46,7 +46,6 @@ import org.projectnessie.model.ContentKey; import org.projectnessie.model.IcebergTable; import org.projectnessie.model.IcebergView; -import org.projectnessie.model.LogResponse; import org.projectnessie.model.LogResponse.LogEntry; import org.projectnessie.model.Operation; import org.projectnessie.model.Operation.Delete; @@ -210,7 +209,7 @@ public void filterCommitLogByTimeRange() throws BaseNessieClientServerException String currentHash = branch.getHash(); createCommits(branch, numAuthors, commitsPerAuthor, currentHash); - List log = commitLog(branch.getName()); + List log = commitLog(branch.getName()); soft.assertThat(log).hasSize(expectedTotalSize); Instant initialCommitTime = log.get(log.size() - 1).getCommitMeta().getCommitTime(); @@ -272,7 +271,7 @@ public void filterCommitLogByProperties() throws BaseNessieClientServerException String currentHash = branch.getHash(); createCommits(branch, numAuthors, commitsPerAuthor, currentHash); - List log = commitLog(branch.getName()); + List log = commitLog(branch.getName()); soft.assertThat(log).hasSize(numAuthors * commitsPerAuthor); log = commitLog(branch.getName(), null, "commit.properties['prop1'] == 'val1'"); @@ -294,13 +293,13 @@ public void filterCommitLogByCommitRange() throws BaseNessieClientServerExceptio String currentHash = branch.getHash(); createCommits(branch, 1, numCommits, currentHash); - List entireLog = commitLog(branch.getName()); + List entireLog = commitLog(branch.getName()); soft.assertThat(entireLog).hasSize(numCommits); // if startHash > endHash, then we return all commits starting from startHash String startHash = entireLog.get(numCommits / 2).getCommitMeta().getHash(); String endHash = entireLog.get(0).getCommitMeta().getHash(); - List log = + List log = treeApi() .getCommitLog( branch.getName(), @@ -341,7 +340,7 @@ public void commitLogPagingAndFilteringByAuthor() throws BaseNessieClientServerE int expectedTotalSize = numAuthors * commits; createCommits(branch, numAuthors, commits, branch.getHash()); - List log = commitLog(branch.getName()); + List log = commitLog(branch.getName()); soft.assertThat(log).hasSize(expectedTotalSize); String author = "author-1"; @@ -522,7 +521,7 @@ public void commitLogExtendedForUnchangedOperation() throws Exception { public void commitLogForNamelessReference() throws BaseNessieClientServerException { Branch branch = createBranch("commitLogForNamelessReference"); String head = createCommits(branch, 1, 5, branch.getHash()); - List log = commitLog(DetachedRef.REF_NAME, MINIMAL, null, head, null); + List log = commitLog(DetachedRef.REF_NAME, MINIMAL, null, head, null); // Verifying size is sufficient to make sure the right log was retrieved assertThat(log).hasSize(5); } diff --git a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestReferences.java b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestReferences.java index e6f63484d3d..d21b6af5b61 100644 --- a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestReferences.java +++ b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestReferences.java @@ -48,7 +48,6 @@ import org.projectnessie.model.IcebergView; import org.projectnessie.model.LogResponse.LogEntry; import org.projectnessie.model.MergeBehavior; -import org.projectnessie.model.Operation; import org.projectnessie.model.Operation.Put; import org.projectnessie.model.Reference; import org.projectnessie.model.ReferenceMetadata; @@ -220,7 +219,7 @@ public void referenceNames(String refNamePart) throws BaseNessieClientServerExce .message("common-merge-ancestor") .properties(ImmutableMap.of("prop1", "val1", "prop2", "val2")) .build(), - Operation.Put.of(ContentKey.of("meep"), meta)) + Put.of(ContentKey.of("meep"), meta)) .getTargetBranch(); String someHash = main.getHash(); diff --git a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/BaseTestServiceImpl.java b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/BaseTestServiceImpl.java index 3bbfd80576c..29ac789da86 100644 --- a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/BaseTestServiceImpl.java +++ b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/BaseTestServiceImpl.java @@ -54,7 +54,6 @@ import org.projectnessie.model.Detached; import org.projectnessie.model.DiffResponse.DiffEntry; import org.projectnessie.model.EntriesResponse; -import org.projectnessie.model.EntriesResponse.Entry; import org.projectnessie.model.FetchOption; import org.projectnessie.model.GetMultipleContentsResponse.ContentWithKey; import org.projectnessie.model.IcebergTable; @@ -229,7 +228,7 @@ protected List allReferences(FetchOption fetchOption, String filter) .getAllReferences(fetchOption, filter, null, new UnlimitedListResponseHandler<>()); } - protected List withoutNamespaces(List entries) { + protected List withoutNamespaces(List entries) { return entries.stream() .filter(e -> e.getType() != Content.Type.NAMESPACE) .collect(Collectors.toList()); @@ -434,7 +433,9 @@ protected Reference getReference(String refName) throws NessieNotFoundException protected Branch ensureNamespacesForKeysExist(Branch targetBranch, ContentKey... keysToCheck) throws NessieConflictException, NessieNotFoundException { Set existingKeys = - entries(targetBranch).stream().map(Entry::getName).collect(Collectors.toSet()); + entries(targetBranch).stream() + .map(EntriesResponse.Entry::getName) + .collect(Collectors.toSet()); Put[] nsToCreate = Arrays.stream(keysToCheck) diff --git a/servers/store/src/test/java/org/projectnessie/server/store/TestStoreWorker.java b/servers/store/src/test/java/org/projectnessie/server/store/TestStoreWorker.java index f5c31e9d8dc..33afc753bea 100644 --- a/servers/store/src/test/java/org/projectnessie/server/store/TestStoreWorker.java +++ b/servers/store/src/test/java/org/projectnessie/server/store/TestStoreWorker.java @@ -57,7 +57,7 @@ void tableMetadataLocationGlobalNotAvailable() { ObjectTypes.Content.newBuilder() .setId(CID) .setIcebergRefState( - ObjectTypes.IcebergRefState.newBuilder() + IcebergRefState.newBuilder() .setSnapshotId(42) .setSchemaId(43) .setSpecId(44) @@ -200,7 +200,7 @@ static Stream requiresGlobalStateModelType() { ObjectTypes.Content.newBuilder() .setId(CID) .setIcebergRefState( - ObjectTypes.IcebergRefState.newBuilder() + IcebergRefState.newBuilder() .setSnapshotId(42) .setSchemaId(43) .setSpecId(44) diff --git a/versioned/persist/adapter/src/main/java/org/projectnessie/versioned/persist/adapter/spi/AbstractDatabaseAdapter.java b/versioned/persist/adapter/src/main/java/org/projectnessie/versioned/persist/adapter/spi/AbstractDatabaseAdapter.java index fa92d635230..70eac0a6f36 100644 --- a/versioned/persist/adapter/src/main/java/org/projectnessie/versioned/persist/adapter/spi/AbstractDatabaseAdapter.java +++ b/versioned/persist/adapter/src/main/java/org/projectnessie/versioned/persist/adapter/spi/AbstractDatabaseAdapter.java @@ -87,7 +87,6 @@ import org.projectnessie.versioned.BranchName; import org.projectnessie.versioned.Diff; import org.projectnessie.versioned.GetNamedRefsParams; -import org.projectnessie.versioned.GetNamedRefsParams.RetrieveOptions; import org.projectnessie.versioned.Hash; import org.projectnessie.versioned.ImmutableKeyDetails; import org.projectnessie.versioned.ImmutableMergeResult; @@ -831,7 +830,8 @@ protected Stream> namedRefsWithDefaultBranchRelatedInf return ref; } - RetrieveOptions retrieveOptions = namedRefsRetrieveOptionsForReference(params, ref); + GetNamedRefsParams.RetrieveOptions retrieveOptions = + namedRefsRetrieveOptionsForReference(params, ref); ReferenceInfo updated = namedRefsRequiresBaseReference(retrieveOptions) diff --git a/versioned/persist/serialize/src/main/java/org/projectnessie/versioned/persist/adapter/serialize/ProtoSerialization.java b/versioned/persist/serialize/src/main/java/org/projectnessie/versioned/persist/adapter/serialize/ProtoSerialization.java index 68c8d387a89..bc31f020d9d 100644 --- a/versioned/persist/serialize/src/main/java/org/projectnessie/versioned/persist/adapter/serialize/ProtoSerialization.java +++ b/versioned/persist/serialize/src/main/java/org/projectnessie/versioned/persist/adapter/serialize/ProtoSerialization.java @@ -34,14 +34,13 @@ import org.projectnessie.versioned.persist.adapter.RefLog; import org.projectnessie.versioned.persist.adapter.RepoDescription; import org.projectnessie.versioned.persist.serialize.AdapterTypes; -import org.projectnessie.versioned.persist.serialize.AdapterTypes.RepoProps; public final class ProtoSerialization { private ProtoSerialization() {} - public static RepoProps toProto(RepoDescription repoDescription) { - RepoProps.Builder proto = + public static AdapterTypes.RepoProps toProto(RepoDescription repoDescription) { + AdapterTypes.RepoProps.Builder proto = AdapterTypes.RepoProps.newBuilder().setRepoVersion(repoDescription.getRepoVersion()); // Must be sorted new TreeMap<>(repoDescription.getProperties()) diff --git a/versioned/persist/serialize/src/test/java/org/projectnessie/versioned/persist/adapter/serialize/TestSerialization.java b/versioned/persist/serialize/src/test/java/org/projectnessie/versioned/persist/adapter/serialize/TestSerialization.java index 683763e6c36..7272e72006a 100644 --- a/versioned/persist/serialize/src/test/java/org/projectnessie/versioned/persist/adapter/serialize/TestSerialization.java +++ b/versioned/persist/serialize/src/test/java/org/projectnessie/versioned/persist/adapter/serialize/TestSerialization.java @@ -211,7 +211,7 @@ static List typeSerialization() { ProtoSerialization::toProto, v -> { try { - return AdapterTypes.RepoProps.parseFrom(v); + return RepoProps.parseFrom(v); } catch (InvalidProtocolBufferException e) { throw new RuntimeException(e); } @@ -326,7 +326,7 @@ public void stableOrderOfRepoDescProps() { props.put(key, value); repoDescription.putProperties(key, value); } - AdapterTypes.RepoProps repoProps = toProto(repoDescription.build()); + RepoProps repoProps = toProto(repoDescription.build()); List expected = props.entrySet().stream() diff --git a/versioned/persist/tx/src/main/java/org/projectnessie/versioned/persist/tx/TxDatabaseAdapter.java b/versioned/persist/tx/src/main/java/org/projectnessie/versioned/persist/tx/TxDatabaseAdapter.java index 3d15ad66de2..9ffebbfa5fb 100644 --- a/versioned/persist/tx/src/main/java/org/projectnessie/versioned/persist/tx/TxDatabaseAdapter.java +++ b/versioned/persist/tx/src/main/java/org/projectnessie/versioned/persist/tx/TxDatabaseAdapter.java @@ -116,7 +116,6 @@ import org.projectnessie.versioned.persist.adapter.spi.Traced; import org.projectnessie.versioned.persist.adapter.spi.TryLoopState; import org.projectnessie.versioned.persist.serialize.AdapterTypes.RefLogEntry; -import org.projectnessie.versioned.persist.serialize.AdapterTypes.RefLogEntry.Operation; import org.projectnessie.versioned.persist.serialize.AdapterTypes.RefLogParents; import org.projectnessie.versioned.persist.serialize.AdapterTypes.RefType; @@ -1673,7 +1672,7 @@ private RefLogEntry writeRefLogEntry( NamedRef ref, RefLogHead refLogHead, Hash commitHash, - Operation operation, + RefLogEntry.Operation operation, long timeInMicros, List sourceHashes) throws ReferenceConflictException { diff --git a/versioned/spi/src/test/java/org/projectnessie/versioned/TestMetricsVersionStore.java b/versioned/spi/src/test/java/org/projectnessie/versioned/TestMetricsVersionStore.java index 746a2b33ea7..d95617f7050 100644 --- a/versioned/spi/src/test/java/org/projectnessie/versioned/TestMetricsVersionStore.java +++ b/versioned/spi/src/test/java/org/projectnessie/versioned/TestMetricsVersionStore.java @@ -426,7 +426,7 @@ static class TestTimer extends AbstractTimer { List recorded = new ArrayList<>(); TestTimer( - Id id, + Meter.Id id, Clock clock, DistributionStatisticConfig distributionStatisticConfig, PauseDetector pauseDetector, diff --git a/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/objtypes/CommitHeadersImpl.java b/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/objtypes/CommitHeadersImpl.java index 46d954c6476..19bf9723444 100644 --- a/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/objtypes/CommitHeadersImpl.java +++ b/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/objtypes/CommitHeadersImpl.java @@ -43,13 +43,13 @@ public BuilderImpl(Map> map) { } @Override - public Builder add(String name, String value) { + public CommitHeaders.Builder add(String name, String value) { nameList(name).add(value); return this; } @Override - public Builder from(CommitHeaders headers) { + public CommitHeaders.Builder from(CommitHeaders headers) { CommitHeadersImpl h = (CommitHeadersImpl) headers; for (Entry> e : h.map.entrySet()) { nameList(e.getKey()).addAll(e.getValue()); diff --git a/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/BaseCommitHelper.java b/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/BaseCommitHelper.java index eed1fd43581..bc556d63a4c 100644 --- a/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/BaseCommitHelper.java +++ b/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/BaseCommitHelper.java @@ -82,7 +82,6 @@ import org.projectnessie.versioned.storage.common.indexes.StoreIndexElement; import org.projectnessie.versioned.storage.common.indexes.StoreKey; import org.projectnessie.versioned.storage.common.logic.CommitLogic; -import org.projectnessie.versioned.storage.common.logic.CommitRetry; import org.projectnessie.versioned.storage.common.logic.CommitRetry.RetryException; import org.projectnessie.versioned.storage.common.logic.ConflictHandler.ConflictResolution; import org.projectnessie.versioned.storage.common.logic.CreateCommit; @@ -692,7 +691,7 @@ void bumpReferencePointer(ObjId newHead, Optional retryState) throws RetryExc try { persist.updateReferencePointer(reference, newHead); } catch (RefConditionFailedException e) { - throw new CommitRetry.RetryException(retryState); + throw new RetryException(retryState); } catch (RefNotFoundException e) { throw new RuntimeException("Internal reference not found", e); } From 6638e7c47c3c0ce96ec6f4f9ba149d12cb11cd7f Mon Sep 17 00:00:00 2001 From: Christopher Lambert Date: Thu, 11 May 2023 17:57:51 +0200 Subject: [PATCH 2/3] review: prefer fixes with shorter code --- .../compatibility/tests/ITUpgradePath.java | 9 ++++---- .../gc/iceberg/mocks/MockManifestEntry.java | 5 +++-- .../gc/iceberg/mocks/MockManifestFile.java | 14 ++++++------- .../spark/extensions/SparkSqlTestBase.java | 21 ++++++++----------- .../services/impl/TreeApiImpl.java | 14 ++++++------- .../services/spi/TreeService.java | 7 ++++--- .../services/impl/TestRefUtil.java | 8 +++---- .../adapter/spi/AbstractDatabaseAdapter.java | 11 +++++----- 8 files changed, 42 insertions(+), 47 deletions(-) diff --git a/compatibility/compatibility-tests/src/intTest/java/org/projectnessie/tools/compatibility/tests/ITUpgradePath.java b/compatibility/compatibility-tests/src/intTest/java/org/projectnessie/tools/compatibility/tests/ITUpgradePath.java index 58b92e90ede..941c0c623b2 100644 --- a/compatibility/compatibility-tests/src/intTest/java/org/projectnessie/tools/compatibility/tests/ITUpgradePath.java +++ b/compatibility/compatibility-tests/src/intTest/java/org/projectnessie/tools/compatibility/tests/ITUpgradePath.java @@ -60,7 +60,7 @@ import org.projectnessie.model.ContentKey; import org.projectnessie.model.EntriesResponse; import org.projectnessie.model.IcebergTable; -import org.projectnessie.model.LogResponse; +import org.projectnessie.model.LogResponse.LogEntry; import org.projectnessie.model.Operation.Delete; import org.projectnessie.model.Operation.Put; import org.projectnessie.model.RefLogResponse.RefLogResponseEntry; @@ -119,8 +119,7 @@ Stream entries(Function commitLog( - Function configurer) + Stream commitLog(Function configurer) throws NessieNotFoundException { if (version.isGreaterThan(Version.parseVersion("0.30.0"))) { return configurer.apply(api.getCommitLog()).stream(); @@ -215,11 +214,11 @@ void commitLog() throws NessieNotFoundException { .allSatisfy( ref -> { String versionFromRef = ref.getName().substring(VERSION_BRANCH_PREFIX.length()); - Stream commitLog = commitLog(b -> b.refName(ref.getName())); + Stream commitLog = commitLog(b -> b.refName(ref.getName())); String commitMessage = "hello world " + versionFromRef; assertThat(commitLog) .hasSize(1) - .map(LogResponse.LogEntry::getCommitMeta) + .map(LogEntry::getCommitMeta) .map(CommitMeta::getMessage) .containsExactly(commitMessage); }) diff --git a/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestEntry.java b/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestEntry.java index 713c7b4ed13..040c5ad0cd2 100644 --- a/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestEntry.java +++ b/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestEntry.java @@ -24,6 +24,7 @@ import org.apache.iceberg.BridgeToIceberg; import org.apache.iceberg.avro.AvroSchemaUtil; import org.apache.iceberg.types.Types; +import org.apache.iceberg.types.Types.StructType; import org.immutables.value.Value; @Value.Immutable @@ -50,7 +51,7 @@ public Status status() { public abstract String filePath(); - public abstract Types.StructType partitionType(); + public abstract StructType partitionType(); // ids for data-file columns are assigned from 1000 static final Types.NestedField STATUS = required(0, "status", Types.IntegerType.get()); @@ -62,7 +63,7 @@ public Status status() { @Override @Value.Auxiliary public Schema getSchema() { - Types.StructType fileSchema = MockManifestFile.fileType(partitionType()); + StructType fileSchema = MockManifestFile.fileType(partitionType()); return AvroSchemaUtil.convert( new org.apache.iceberg.Schema( STATUS, SNAPSHOT_ID, SEQUENCE_NUMBER, required(DATA_FILE_ID, "data_file", fileSchema)), diff --git a/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestFile.java b/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestFile.java index b60f49cc964..4406c2f70e5 100644 --- a/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestFile.java +++ b/gc/gc-iceberg-mock/src/main/java/org/projectnessie/gc/iceberg/mocks/MockManifestFile.java @@ -38,7 +38,7 @@ import org.apache.iceberg.avro.AvroSchemaUtil; import org.apache.iceberg.io.FileAppender; import org.apache.iceberg.io.OutputFile; -import org.apache.iceberg.types.Types; +import org.apache.iceberg.types.Types.StructType; import org.immutables.value.Value; @Value.Immutable @@ -142,7 +142,7 @@ public List manifestEntries() { MockPartitionSpec mockPartitionSpec = mockTableMeta.partitionSpec(0); org.apache.iceberg.Schema icebergSchema = mockTableMeta.schema(0).toSchema(); PartitionSpec partitionSpec = mockPartitionSpec.toPartitionSpec(icebergSchema); - Types.StructType partitionType = partitionSpec.partitionType(); + StructType partitionType = partitionSpec.partitionType(); return IntStream.range(0, numEntries()) .mapToObj( @@ -163,7 +163,7 @@ public void write(OutputFile output) { MockPartitionSpec mockPartitionSpec = mockTableMeta.partitionSpec(0); org.apache.iceberg.Schema icebergSchema = mockTableMeta.schema(0).toSchema(); PartitionSpec partitionSpec = mockPartitionSpec.toPartitionSpec(icebergSchema); - Types.StructType partitionType = partitionSpec.partitionType(); + StructType partitionType = partitionSpec.partitionType(); try { String partitionSpecJson = PartitionSpecParser.toJson(partitionSpec); @@ -192,11 +192,11 @@ public void write(OutputFile output) { } } - static org.apache.iceberg.Schema entrySchema(Types.StructType partitionType) { + static org.apache.iceberg.Schema entrySchema(StructType partitionType) { return wrapFileSchema(fileType(partitionType)); } - static org.apache.iceberg.Schema wrapFileSchema(Types.StructType fileSchema) { + static org.apache.iceberg.Schema wrapFileSchema(StructType fileSchema) { // this is used to build projection schemas return new org.apache.iceberg.Schema( MockManifestEntry.STATUS, @@ -205,8 +205,8 @@ static org.apache.iceberg.Schema wrapFileSchema(Types.StructType fileSchema) { required(MockManifestEntry.DATA_FILE_ID, "data_file", fileSchema)); } - static Types.StructType fileType(Types.StructType partitionType) { - return Types.StructType.of( + static StructType fileType(StructType partitionType) { + return StructType.of( DataFile.CONTENT.asRequired(), DataFile.FILE_PATH, DataFile.FILE_FORMAT, diff --git a/integrations/spark-extensions-basetests/src/main/java/org/projectnessie/spark/extensions/SparkSqlTestBase.java b/integrations/spark-extensions-basetests/src/main/java/org/projectnessie/spark/extensions/SparkSqlTestBase.java index 7893c1ad435..44f3c254958 100644 --- a/integrations/spark-extensions-basetests/src/main/java/org/projectnessie/spark/extensions/SparkSqlTestBase.java +++ b/integrations/spark-extensions-basetests/src/main/java/org/projectnessie/spark/extensions/SparkSqlTestBase.java @@ -50,7 +50,8 @@ import org.projectnessie.model.ImmutableCommitMeta; import org.projectnessie.model.ImmutableOperations; import org.projectnessie.model.Namespace; -import org.projectnessie.model.Operation; +import org.projectnessie.model.Operation.Delete; +import org.projectnessie.model.Operation.Put; import org.projectnessie.model.Operations; import org.projectnessie.model.Reference; import org.projectnessie.model.Tag; @@ -105,14 +106,13 @@ protected void setupSparkAndApi(TestInfo testInfo) api.commitMultipleOperations() .branch(initialDefaultBranch) .commitMeta(CommitMeta.fromMessage("INFRA: initial commit")) - .operation( - Operation.Put.of(ContentKey.of("dummy"), IcebergTable.of("foo", 1, 2, 3, 4))) + .operation(Put.of(ContentKey.of("dummy"), IcebergTable.of("foo", 1, 2, 3, 4))) .commit(); initialDefaultBranch = api.commitMultipleOperations() .branch(initialDefaultBranch) .commitMeta(CommitMeta.fromMessage("INFRA: common ancestor")) - .operation(Operation.Delete.of(ContentKey.of("dummy"))) + .operation(Delete.of(ContentKey.of("dummy"))) .commit(); first = false; } @@ -298,14 +298,13 @@ protected List commitAndReturnLog(String branch, String ini content != null ? ImmutableOperations.builder() .addOperations( - Operation.Put.of( - key, IcebergTable.of("foo", 42, 42, 42, 42, content.getId()), content)) + Put.of(key, IcebergTable.of("foo", 42, 42, 42, 42, content.getId()), content)) .commitMeta(cm1) .build() : ImmutableOperations.builder() .addOperations( - Operation.Put.of(ContentKey.of("table"), Namespace.of("table")), - Operation.Put.of(key, IcebergTable.of("foo", 42, 42, 42, 42))) + Put.of(ContentKey.of("table"), Namespace.of("table")), + Put.of(key, IcebergTable.of("foo", 42, 42, 42, 42))) .commitMeta(cm1) .build(); @@ -322,8 +321,7 @@ protected List commitAndReturnLog(String branch, String ini Operations ops2 = ImmutableOperations.builder() .addOperations( - Operation.Put.of( - key, IcebergTable.of("bar", 42, 42, 42, 42, content.getId()), content)) + Put.of(key, IcebergTable.of("bar", 42, 42, 42, 42, content.getId()), content)) .commitMeta(cm2) .build(); @@ -340,8 +338,7 @@ protected List commitAndReturnLog(String branch, String ini Operations ops3 = ImmutableOperations.builder() .addOperations( - Operation.Put.of( - key, IcebergTable.of("baz", 42, 42, 42, 42, content.getId()), content)) + Put.of(key, IcebergTable.of("baz", 42, 42, 42, 42, content.getId()), content)) .commitMeta(cm3) .build(); diff --git a/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java b/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java index a5ea55968f4..a24c811b2f3 100644 --- a/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java +++ b/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java @@ -91,6 +91,7 @@ import org.projectnessie.model.Operation; import org.projectnessie.model.Operations; import org.projectnessie.model.Reference; +import org.projectnessie.model.Reference.ReferenceType; import org.projectnessie.model.ReferenceMetadata; import org.projectnessie.model.Tag; import org.projectnessie.model.Validation; @@ -247,11 +248,11 @@ public Reference getReferenceByName(String refName, FetchOption fetchOption) @Override public Reference createReference( - String refName, Reference.ReferenceType type, String targetHash, String sourceRefName) + String refName, ReferenceType type, String targetHash, String sourceRefName) throws NessieNotFoundException, NessieConflictException { Validation.validateForbiddenReferenceName(refName); NamedRef namedReference = toNamedRef(type, refName); - if (type == Reference.ReferenceType.TAG && targetHash == null) { + if (type == ReferenceType.TAG && targetHash == null) { throw new IllegalArgumentException( "Tag-creation requires a target named-reference and hash."); } @@ -264,7 +265,7 @@ public Reference createReference( // If the default-branch does not exist and hashOnRef points to the "beginning of time", // then do not throw a NessieNotFoundException, but re-create the default branch. In all // cases, re-throw the exception. - if (!(Reference.ReferenceType.BRANCH.equals(type) + if (!(ReferenceType.BRANCH.equals(type) && refName.equals(getConfig().getDefaultBranch()) && (null == targetHash || getStore().noAncestorHash().asString().equals(targetHash)))) { throw e; @@ -291,10 +292,7 @@ public Branch getDefaultBranch() throws NessieNotFoundException { @Override public Reference assignReference( - Reference.ReferenceType referenceType, - String referenceName, - String expectedHash, - Reference assignTo) + ReferenceType referenceType, String referenceName, String expectedHash, Reference assignTo) throws NessieNotFoundException, NessieConflictException { try { ReferenceInfo resolved = @@ -325,7 +323,7 @@ public Reference assignReference( @Override public Reference deleteReference( - Reference.ReferenceType referenceType, String referenceName, String expectedHash) + ReferenceType referenceType, String referenceName, String expectedHash) throws NessieConflictException, NessieNotFoundException { try { ReferenceInfo resolved = diff --git a/servers/services/src/main/java/org/projectnessie/services/spi/TreeService.java b/servers/services/src/main/java/org/projectnessie/services/spi/TreeService.java index 9048c4125da..37060b70486 100644 --- a/servers/services/src/main/java/org/projectnessie/services/spi/TreeService.java +++ b/servers/services/src/main/java/org/projectnessie/services/spi/TreeService.java @@ -36,6 +36,7 @@ import org.projectnessie.model.MergeResponse; import org.projectnessie.model.Operations; import org.projectnessie.model.Reference; +import org.projectnessie.model.Reference.ReferenceType; import org.projectnessie.model.Validation; import org.projectnessie.versioned.NamedRef; import org.projectnessie.versioned.WithHash; @@ -81,7 +82,7 @@ Reference createReference( regexp = Validation.REF_NAME_REGEX, message = Validation.REF_NAME_MESSAGE) String refName, - Reference.ReferenceType type, + ReferenceType type, @Valid @jakarta.validation.Valid @Pattern(regexp = Validation.HASH_REGEX, message = Validation.HASH_MESSAGE) @@ -99,7 +100,7 @@ Reference createReference( throws NessieNotFoundException, NessieConflictException; Reference assignReference( - Reference.ReferenceType referenceType, + ReferenceType referenceType, @Valid @jakarta.validation.Valid @NotNull @@ -120,7 +121,7 @@ Reference assignReference( throws NessieNotFoundException, NessieConflictException; Reference deleteReference( - Reference.ReferenceType referenceType, + ReferenceType referenceType, @Valid @jakarta.validation.Valid @NotNull diff --git a/servers/services/src/test/java/org/projectnessie/services/impl/TestRefUtil.java b/servers/services/src/test/java/org/projectnessie/services/impl/TestRefUtil.java index 4494bf68145..aa6c046c71f 100644 --- a/servers/services/src/test/java/org/projectnessie/services/impl/TestRefUtil.java +++ b/servers/services/src/test/java/org/projectnessie/services/impl/TestRefUtil.java @@ -24,6 +24,7 @@ import org.projectnessie.model.Branch; import org.projectnessie.model.Detached; import org.projectnessie.model.Reference; +import org.projectnessie.model.Reference.ReferenceType; import org.projectnessie.model.ReferenceMetadata; import org.projectnessie.model.Tag; import org.projectnessie.versioned.BranchName; @@ -73,7 +74,7 @@ public ReferenceMetadata getMetadata() { } @Override - public Reference.ReferenceType getType() { + public ReferenceType getType() { return null; } })) @@ -84,10 +85,9 @@ public Reference.ReferenceType getType() { @Test void toNamedRefTyped() { - assertThat(RefUtil.toNamedRef(Reference.ReferenceType.BRANCH, REF_NAME)) + assertThat(RefUtil.toNamedRef(ReferenceType.BRANCH, REF_NAME)) .isEqualTo(BranchName.of(REF_NAME)); - assertThat(RefUtil.toNamedRef(Reference.ReferenceType.TAG, REF_NAME)) - .isEqualTo(TagName.of(REF_NAME)); + assertThat(RefUtil.toNamedRef(ReferenceType.TAG, REF_NAME)).isEqualTo(TagName.of(REF_NAME)); } @Test diff --git a/versioned/persist/adapter/src/main/java/org/projectnessie/versioned/persist/adapter/spi/AbstractDatabaseAdapter.java b/versioned/persist/adapter/src/main/java/org/projectnessie/versioned/persist/adapter/spi/AbstractDatabaseAdapter.java index 70eac0a6f36..5b0cde4f6b2 100644 --- a/versioned/persist/adapter/src/main/java/org/projectnessie/versioned/persist/adapter/spi/AbstractDatabaseAdapter.java +++ b/versioned/persist/adapter/src/main/java/org/projectnessie/versioned/persist/adapter/spi/AbstractDatabaseAdapter.java @@ -87,6 +87,7 @@ import org.projectnessie.versioned.BranchName; import org.projectnessie.versioned.Diff; import org.projectnessie.versioned.GetNamedRefsParams; +import org.projectnessie.versioned.GetNamedRefsParams.RetrieveOptions; import org.projectnessie.versioned.Hash; import org.projectnessie.versioned.ImmutableKeyDetails; import org.projectnessie.versioned.ImmutableMergeResult; @@ -746,8 +747,7 @@ protected static boolean namedRefsRequiresBaseReference(GetNamedRefsParams param || namedRefsRequiresBaseReference(params.getTagRetrieveOptions()); } - protected static boolean namedRefsRequiresBaseReference( - GetNamedRefsParams.RetrieveOptions retrieveOptions) { + protected static boolean namedRefsRequiresBaseReference(RetrieveOptions retrieveOptions) { return retrieveOptions.isComputeAheadBehind() || retrieveOptions.isComputeCommonAncestor(); } @@ -756,12 +756,12 @@ protected static boolean namedRefsAnyRetrieves(GetNamedRefsParams params) { || params.getTagRetrieveOptions().isRetrieve(); } - protected static GetNamedRefsParams.RetrieveOptions namedRefsRetrieveOptionsForReference( + protected static RetrieveOptions namedRefsRetrieveOptionsForReference( GetNamedRefsParams params, ReferenceInfo ref) { return namedRefsRetrieveOptionsForReference(params, ref.getNamedRef()); } - protected static GetNamedRefsParams.RetrieveOptions namedRefsRetrieveOptionsForReference( + protected static RetrieveOptions namedRefsRetrieveOptionsForReference( GetNamedRefsParams params, NamedRef ref) { if (ref instanceof BranchName) { return params.getBranchRetrieveOptions(); @@ -830,8 +830,7 @@ protected Stream> namedRefsWithDefaultBranchRelatedInf return ref; } - GetNamedRefsParams.RetrieveOptions retrieveOptions = - namedRefsRetrieveOptionsForReference(params, ref); + RetrieveOptions retrieveOptions = namedRefsRetrieveOptionsForReference(params, ref); ReferenceInfo updated = namedRefsRequiresBaseReference(retrieveOptions) From a57de95a6f9c94a615818836bbb54dee18eae874 Mon Sep 17 00:00:00 2001 From: Christopher Lambert Date: Fri, 12 May 2023 09:02:46 +0200 Subject: [PATCH 3/3] review: Entry is ok --- .../jaxrs/tests/BaseTestNessieApi.java | 20 +++++++------- .../services/impl/TreeApiImpl.java | 21 +++++++-------- .../services/impl/BaseTestServiceImpl.java | 26 ++++++++----------- 3 files changed, 31 insertions(+), 36 deletions(-) diff --git a/servers/jax-rs-tests/src/main/java/org/projectnessie/jaxrs/tests/BaseTestNessieApi.java b/servers/jax-rs-tests/src/main/java/org/projectnessie/jaxrs/tests/BaseTestNessieApi.java index 030e5386cfb..5c6d0e59d33 100644 --- a/servers/jax-rs-tests/src/main/java/org/projectnessie/jaxrs/tests/BaseTestNessieApi.java +++ b/servers/jax-rs-tests/src/main/java/org/projectnessie/jaxrs/tests/BaseTestNessieApi.java @@ -83,6 +83,7 @@ import org.projectnessie.model.DiffResponse; import org.projectnessie.model.DiffResponse.DiffEntry; import org.projectnessie.model.EntriesResponse; +import org.projectnessie.model.EntriesResponse.Entry; import org.projectnessie.model.GetMultipleContentsResponse; import org.projectnessie.model.GetNamespacesResponse; import org.projectnessie.model.IcebergTable; @@ -470,7 +471,7 @@ public void commitMergeTransplant() throws Exception { .hasSize(4); soft.assertThat(api().getEntries().reference(branch).get().getEntries()) - .extracting(EntriesResponse.Entry::getName) + .extracting(Entry::getName) .containsExactlyInAnyOrder( ContentKey.of("a"), ContentKey.of("b"), @@ -480,7 +481,7 @@ public void commitMergeTransplant() throws Exception { soft.assertThat(api().getCommitLog().refName(main.getName()).get().getLogEntries()).hasSize(2); soft.assertThat(api().getEntries().reference(main).get().getEntries()) - .extracting(EntriesResponse.Entry::getName) + .extracting(Entry::getName) .containsExactly(ContentKey.of("a"), ContentKey.of("b")); Reference main2; @@ -527,7 +528,7 @@ public void commitMergeTransplant() throws Exception { } soft.assertThat(api().getEntries().reference(main2).get().getEntries()) - .extracting(EntriesResponse.Entry::getName) + .extracting(Entry::getName) .containsExactlyInAnyOrder( ContentKey.of("a"), ContentKey.of("b"), @@ -536,7 +537,7 @@ public void commitMergeTransplant() throws Exception { ContentKey.of("b", "b")); soft.assertThat(api().getEntries().reference(otherBranch).get().getEntries()) - .extracting(EntriesResponse.Entry::getName) + .extracting(Entry::getName) .containsExactly(ContentKey.of("a"), ContentKey.of("b")); api() .transplantCommitsIntoBranch() @@ -545,7 +546,7 @@ public void commitMergeTransplant() throws Exception { .branch(otherBranch) .transplant(); soft.assertThat(api().getEntries().refName(otherBranch.getName()).get().getEntries()) - .extracting(EntriesResponse.Entry::getName) + .extracting(Entry::getName) .containsExactlyInAnyOrder( ContentKey.of("a"), ContentKey.of("b"), @@ -961,15 +962,15 @@ public void entries() throws Exception { if (isV2()) { soft.assertThat(response.getEffectiveReference()).isEqualTo(main); soft.assertThat(response.getEntries()) - .extracting(EntriesResponse.Entry::getContent) + .extracting(Entry::getContent) .doesNotContainNull() .isNotEmpty(); } - List notPaged = response.getEntries(); + List notPaged = response.getEntries(); soft.assertThat(notPaged).hasSize(10); if (pagingSupported(api().getEntries())) { - List all = new ArrayList<>(); + List all = new ArrayList<>(); String token = null; for (int i = 0; i < 10; i++) { EntriesResponse resp = @@ -1439,8 +1440,7 @@ public void fetchEntriesByNamelessReference() throws BaseNessieClientServerExcep .operation(Put.of(b, tb)) .commitMeta(CommitMeta.fromMessage("commit 1")) .commit(); - List entries = - api().getEntries().hashOnRef(branch.getHash()).get().getEntries(); + List entries = api().getEntries().hashOnRef(branch.getHash()).get().getEntries(); soft.assertThat(entries) .map(e -> immutableEntry(e.getName(), e.getType())) .containsExactlyInAnyOrder( diff --git a/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java b/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java index a24c811b2f3..8fd3c69713d 100644 --- a/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java +++ b/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java @@ -75,7 +75,7 @@ import org.projectnessie.model.Content; import org.projectnessie.model.ContentKey; import org.projectnessie.model.Detached; -import org.projectnessie.model.EntriesResponse; +import org.projectnessie.model.EntriesResponse.Entry; import org.projectnessie.model.FetchOption; import org.projectnessie.model.ImmutableCommitMeta; import org.projectnessie.model.ImmutableCommitResponse; @@ -776,7 +776,7 @@ public R getEntries( String filter, String pagingToken, boolean withContent, - PagedResponseHandler pagedResponseHandler, + PagedResponseHandler pagedResponseHandler, Consumer> effectiveReference) throws NessieNotFoundException { WithHash refWithHash = namedRefWithHashOrThrow(namedRef, hashOnRef); @@ -820,10 +820,10 @@ protected Set checksForEntry(KeyEntry entry) { } Content c = key.getContent(); - EntriesResponse.Entry entry = + Entry entry = c != null - ? EntriesResponse.Entry.entry(key.getKey(), key.getType(), c) - : EntriesResponse.Entry.entry(key.getKey(), key.getType(), key.getContentId()); + ? Entry.entry(key.getKey(), key.getType(), c) + : Entry.entry(key.getKey(), key.getType(), key.getContentId()); entry = maybeTruncateToDepth(entry, depth); @@ -844,10 +844,10 @@ protected Set checksForEntry(KeyEntry entry) { } Content c = key.getContent(); - EntriesResponse.Entry entry = + Entry entry = c != null - ? EntriesResponse.Entry.entry(key.getKey(), key.getType(), c) - : EntriesResponse.Entry.entry(key.getKey(), key.getType(), key.getContentId()); + ? Entry.entry(key.getKey(), key.getType(), c) + : Entry.entry(key.getKey(), key.getType(), key.getContentId()); if (!pagedResponseHandler.addEntry(entry)) { pagedResponseHandler.hasMore(authz.tokenForCurrent()); @@ -862,14 +862,13 @@ protected Set checksForEntry(KeyEntry entry) { } } - private static EntriesResponse.Entry maybeTruncateToDepth( - EntriesResponse.Entry entry, int depth) { + private static Entry maybeTruncateToDepth(Entry entry, int depth) { List nameElements = entry.getName().getElements(); boolean truncateToNamespace = nameElements.size() > depth; if (truncateToNamespace) { // implicit namespace entry at target depth (virtual parent of real entry) ContentKey namespaceKey = ContentKey.of(nameElements.subList(0, depth)); - return EntriesResponse.Entry.entry(namespaceKey, Content.Type.NAMESPACE); + return Entry.entry(namespaceKey, Content.Type.NAMESPACE); } return entry; } diff --git a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/BaseTestServiceImpl.java b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/BaseTestServiceImpl.java index 29ac789da86..7fea41d9522 100644 --- a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/BaseTestServiceImpl.java +++ b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/BaseTestServiceImpl.java @@ -53,7 +53,7 @@ import org.projectnessie.model.DeltaLakeTable; import org.projectnessie.model.Detached; import org.projectnessie.model.DiffResponse.DiffEntry; -import org.projectnessie.model.EntriesResponse; +import org.projectnessie.model.EntriesResponse.Entry; import org.projectnessie.model.FetchOption; import org.projectnessie.model.GetMultipleContentsResponse.ContentWithKey; import org.projectnessie.model.IcebergTable; @@ -228,28 +228,26 @@ protected List allReferences(FetchOption fetchOption, String filter) .getAllReferences(fetchOption, filter, null, new UnlimitedListResponseHandler<>()); } - protected List withoutNamespaces(List entries) { + protected List withoutNamespaces(List entries) { return entries.stream() .filter(e -> e.getType() != Content.Type.NAMESPACE) .collect(Collectors.toList()); } - protected List entries(Reference reference) - throws NessieNotFoundException { + protected List entries(Reference reference) throws NessieNotFoundException { return entries(reference.getName(), reference.getHash(), null, null, false); } - protected List entries(String refName, String hashOnRef) - throws NessieNotFoundException { + protected List entries(String refName, String hashOnRef) throws NessieNotFoundException { return entries(refName, hashOnRef, null, null, false); } - protected List entries( - Reference reference, Integer namespaceDepth, String filter) throws NessieNotFoundException { + protected List entries(Reference reference, Integer namespaceDepth, String filter) + throws NessieNotFoundException { return entries(reference.getName(), reference.getHash(), namespaceDepth, filter, false); } - protected List entries( + protected List entries( String refName, String hashOnRef, Integer namespaceDepth, String filter, boolean withContent) throws NessieNotFoundException { return treeApi() @@ -339,7 +337,7 @@ protected List pagedReferences( return completeLog; } - protected List pagedEntries( + protected List pagedEntries( Reference ref, String filter, int pageSize, @@ -348,11 +346,11 @@ protected List pagedEntries( boolean withContent) throws NessieNotFoundException { - List completeLog = new ArrayList<>(); + List completeLog = new ArrayList<>(); String token = null; for (int i = 0; ; i++) { AtomicReference nextToken = new AtomicReference<>(); - List page = + List page = treeApi() .getEntries( ref.getName(), @@ -433,9 +431,7 @@ protected Reference getReference(String refName) throws NessieNotFoundException protected Branch ensureNamespacesForKeysExist(Branch targetBranch, ContentKey... keysToCheck) throws NessieConflictException, NessieNotFoundException { Set existingKeys = - entries(targetBranch).stream() - .map(EntriesResponse.Entry::getName) - .collect(Collectors.toSet()); + entries(targetBranch).stream().map(Entry::getName).collect(Collectors.toSet()); Put[] nsToCreate = Arrays.stream(keysToCheck)