From 4f9b28d748e25dcae7a8763f65d685acf2aa82d4 Mon Sep 17 00:00:00 2001 From: Tom Papke <43675490+Toemmsche@users.noreply.github.com> Date: Thu, 13 Jul 2023 15:33:53 +0200 Subject: [PATCH] [LWMeta] Add Conjure metadata types and conversion methods (#6653) [LWMeta] Add Conjure metadata types and conversion methods --- .../com/palantir/util/IndexEncodingUtils.java | 4 +- lock-api/build.gradle | 2 + .../ConjureLockRequestMetadataUtils.java | 176 ++++++++++++++++++ .../ConjureLockRequestMetadataUtilsTest.java | 155 +++++++++++++++ .../src/main/conjure/timelock-api.yml | 38 ++++ 5 files changed, 372 insertions(+), 3 deletions(-) create mode 100644 lock-api/src/main/java/com/palantir/lock/watch/ConjureLockRequestMetadataUtils.java create mode 100644 lock-api/src/test/java/com/palantir/lock/watch/ConjureLockRequestMetadataUtilsTest.java diff --git a/atlasdb-commons/src/main/java/com/palantir/util/IndexEncodingUtils.java b/atlasdb-commons/src/main/java/com/palantir/util/IndexEncodingUtils.java index e20280a5d10..c2fa8c43f4d 100644 --- a/atlasdb-commons/src/main/java/com/palantir/util/IndexEncodingUtils.java +++ b/atlasdb-commons/src/main/java/com/palantir/util/IndexEncodingUtils.java @@ -17,7 +17,6 @@ package com.palantir.util; import com.fasterxml.jackson.annotation.JsonIgnoreType; -import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.palantir.logsafe.Preconditions; @@ -115,8 +114,7 @@ public static Map decode( return keyToValue; } - @VisibleForTesting - static KeyListChecksum computeChecksum( + public static KeyListChecksum computeChecksum( ChecksumType checksumType, List keyList) { switch (checksumType) { case CRC32_OF_DETERMINISTIC_HASHCODE: { diff --git a/lock-api/build.gradle b/lock-api/build.gradle index c201a750377..b9d68aed0eb 100644 --- a/lock-api/build.gradle +++ b/lock-api/build.gradle @@ -44,8 +44,10 @@ dependencies { testImplementation 'com.fasterxml.jackson.datatype:jackson-datatype-jsr310' testImplementation 'com.google.guava:guava' testImplementation 'com.palantir.common:streams' + testImplementation 'com.palantir.safe-logging:preconditions-assertj' testImplementation 'io.dropwizard.metrics:metrics-core' testImplementation 'org.slf4j:slf4j-api' + testImplementation project(':atlasdb-api') testImplementation project(':lock-api-objects') testImplementation project(':lock-conjure-api:lock-conjure-api-objects') testImplementation project(':timelock-api:timelock-api-objects') diff --git a/lock-api/src/main/java/com/palantir/lock/watch/ConjureLockRequestMetadataUtils.java b/lock-api/src/main/java/com/palantir/lock/watch/ConjureLockRequestMetadataUtils.java new file mode 100644 index 00000000000..04139c5c6a9 --- /dev/null +++ b/lock-api/src/main/java/com/palantir/lock/watch/ConjureLockRequestMetadataUtils.java @@ -0,0 +1,176 @@ +/* + * (c) Copyright 2023 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.lock.watch; + +import com.google.common.annotations.VisibleForTesting; +import com.palantir.atlasdb.timelock.api.ConjureChangeMetadata; +import com.palantir.atlasdb.timelock.api.ConjureCreatedChangeMetadata; +import com.palantir.atlasdb.timelock.api.ConjureDeletedChangeMetadata; +import com.palantir.atlasdb.timelock.api.ConjureLockDescriptorListChecksum; +import com.palantir.atlasdb.timelock.api.ConjureLockRequestMetadata; +import com.palantir.atlasdb.timelock.api.ConjureUnchangedChangeMetadata; +import com.palantir.atlasdb.timelock.api.ConjureUpdatedChangeMetadata; +import com.palantir.common.streams.KeyedStream; +import com.palantir.conjure.java.lib.Bytes; +import com.palantir.lock.LockDescriptor; +import com.palantir.lock.watch.ChangeMetadata.Created; +import com.palantir.lock.watch.ChangeMetadata.Deleted; +import com.palantir.lock.watch.ChangeMetadata.Unchanged; +import com.palantir.lock.watch.ChangeMetadata.Updated; +import com.palantir.logsafe.SafeArg; +import com.palantir.logsafe.Unsafe; +import com.palantir.logsafe.logger.SafeLogger; +import com.palantir.logsafe.logger.SafeLoggerFactory; +import com.palantir.util.IndexEncodingUtils; +import com.palantir.util.IndexEncodingUtils.ChecksumType; +import com.palantir.util.IndexEncodingUtils.IndexEncodingResult; +import com.palantir.util.IndexEncodingUtils.KeyListChecksum; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import org.immutables.value.Value; + +public final class ConjureLockRequestMetadataUtils { + private ConjureLockRequestMetadataUtils() {} + + @VisibleForTesting + static final ChecksumType DEFAULT_CHECKSUM_TYPE = ChecksumType.CRC32_OF_DETERMINISTIC_HASHCODE; + + private static final SafeLogger log = SafeLoggerFactory.get(ConjureLockRequestMetadataUtils.class); + + public static ConjureMetadataConversionResult toConjureIndexEncoded( + Set lockDescriptors, LockRequestMetadata metadata) { + IndexEncodingResult encoded = IndexEncodingUtils.encode( + lockDescriptors, + metadata.lockDescriptorToChangeMetadata(), + changeMetadata -> changeMetadata.accept(ChangeMetadataToConjureVisitor.INSTANCE), + DEFAULT_CHECKSUM_TYPE); + KeyListChecksum checksum = encoded.keyListChecksum(); + ConjureLockRequestMetadata conjureLockRequestMetadata = ConjureLockRequestMetadata.builder() + .indexToChangeMetadata(encoded.indexToValue()) + .lockListChecksum(checksumToConjure(checksum)) + .build(); + return ImmutableConjureMetadataConversionResult.builder() + .lockList(encoded.keyList()) + .conjureMetadata(conjureLockRequestMetadata) + .build(); + } + + public static LockRequestMetadata fromConjureIndexEncoded(ConjureMetadataConversionResult conversionResult) { + List keyList = conversionResult.lockList(); + ConjureLockRequestMetadata conjureMetadata = conversionResult.conjureMetadata(); + KeyListChecksum checksum = + checksumFromConjure(conversionResult.conjureMetadata().getLockListChecksum()); + IndexEncodingResult encoded = + IndexEncodingResult.builder() + .keyList(keyList) + .indexToValue(conjureMetadata.getIndexToChangeMetadata()) + .keyListChecksum(checksum) + .build(); + Map> optChangeMetadata = IndexEncodingUtils.decode( + encoded, + conjureChangeMetadata -> conjureChangeMetadata.accept(ChangeMetadataFromConjureVisitor.INSTANCE)); + Map changeMetadata = KeyedStream.ofEntries( + optChangeMetadata.entrySet().stream()) + .flatMap(Optional::stream) + .collectToMap(); + return LockRequestMetadata.of(changeMetadata); + } + + private static ConjureLockDescriptorListChecksum checksumToConjure(KeyListChecksum checksum) { + return ConjureLockDescriptorListChecksum.of(checksum.type().getId(), Bytes.from(checksum.value())); + } + + private static KeyListChecksum checksumFromConjure(ConjureLockDescriptorListChecksum conjureChecksum) { + return KeyListChecksum.of( + ChecksumType.valueOf(conjureChecksum.getTypeId()), + conjureChecksum.getValue().asNewByteArray()); + } + + @Unsafe + @Value.Immutable + public interface ConjureMetadataConversionResult { + List lockList(); + + ConjureLockRequestMetadata conjureMetadata(); + + static ImmutableConjureMetadataConversionResult.Builder builder() { + return ImmutableConjureMetadataConversionResult.builder(); + } + } + + private enum ChangeMetadataToConjureVisitor implements ChangeMetadata.Visitor { + INSTANCE; + + @Override + public ConjureChangeMetadata visit(Unchanged unchanged) { + return ConjureChangeMetadata.unchanged(ConjureUnchangedChangeMetadata.of()); + } + + @Override + public ConjureChangeMetadata visit(Updated updated) { + return ConjureChangeMetadata.updated( + ConjureUpdatedChangeMetadata.of(Bytes.from(updated.oldValue()), Bytes.from(updated.newValue()))); + } + + @Override + public ConjureChangeMetadata visit(Deleted deleted) { + return ConjureChangeMetadata.deleted(ConjureDeletedChangeMetadata.of(Bytes.from(deleted.oldValue()))); + } + + @Override + public ConjureChangeMetadata visit(Created created) { + return ConjureChangeMetadata.created(ConjureCreatedChangeMetadata.of(Bytes.from(created.newValue()))); + } + } + + private enum ChangeMetadataFromConjureVisitor implements ConjureChangeMetadata.Visitor> { + INSTANCE; + + @Override + public Optional visitUnchanged(ConjureUnchangedChangeMetadata unchanged) { + return Optional.of(ChangeMetadata.unchanged()); + } + + @Override + public Optional visitUpdated(ConjureUpdatedChangeMetadata updated) { + return Optional.of(ChangeMetadata.updated( + updated.getOldValue().asNewByteArray(), + updated.getNewValue().asNewByteArray())); + } + + @Override + public Optional visitDeleted(ConjureDeletedChangeMetadata deleted) { + return Optional.of(ChangeMetadata.deleted(deleted.getOldValue().asNewByteArray())); + } + + @Override + public Optional visitCreated(ConjureCreatedChangeMetadata created) { + return Optional.of(ChangeMetadata.created(created.getNewValue().asNewByteArray())); + } + + @Override + public Optional visitUnknown(String unknownType) { + log.trace( + "Encountered an unknown ConjureChangeMetadata type. This is likely a new type that was added in a" + + " future version. This ChangeMetadata will be discarded", + SafeArg.of("unknownType", unknownType)); + return Optional.empty(); + } + } +} diff --git a/lock-api/src/test/java/com/palantir/lock/watch/ConjureLockRequestMetadataUtilsTest.java b/lock-api/src/test/java/com/palantir/lock/watch/ConjureLockRequestMetadataUtilsTest.java new file mode 100644 index 00000000000..c0d1025cfe5 --- /dev/null +++ b/lock-api/src/test/java/com/palantir/lock/watch/ConjureLockRequestMetadataUtilsTest.java @@ -0,0 +1,155 @@ +/* + * (c) Copyright 2023 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.lock.watch; + +import static com.palantir.logsafe.testing.Assertions.assertThatLoggableExceptionThrownBy; +import static org.assertj.core.api.Assertions.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.palantir.atlasdb.encoding.PtBytes; +import com.palantir.atlasdb.timelock.api.ConjureChangeMetadata; +import com.palantir.atlasdb.timelock.api.ConjureCreatedChangeMetadata; +import com.palantir.atlasdb.timelock.api.ConjureDeletedChangeMetadata; +import com.palantir.atlasdb.timelock.api.ConjureLockDescriptorListChecksum; +import com.palantir.atlasdb.timelock.api.ConjureLockRequestMetadata; +import com.palantir.atlasdb.timelock.api.ConjureUnchangedChangeMetadata; +import com.palantir.atlasdb.timelock.api.ConjureUpdatedChangeMetadata; +import com.palantir.conjure.java.lib.Bytes; +import com.palantir.lock.LockDescriptor; +import com.palantir.lock.StringLockDescriptor; +import com.palantir.lock.watch.ConjureLockRequestMetadataUtils.ConjureMetadataConversionResult; +import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; +import com.palantir.util.IndexEncodingUtils; +import com.palantir.util.IndexEncodingUtils.KeyListChecksum; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import org.junit.BeforeClass; +import org.junit.Test; + +public class ConjureLockRequestMetadataUtilsTest { + private static final LockDescriptor LOCK_1 = StringLockDescriptor.of("lock1"); + private static final LockDescriptor LOCK_2 = StringLockDescriptor.of("lock2"); + private static final LockDescriptor LOCK_3 = StringLockDescriptor.of("lock3"); + private static final LockDescriptor LOCK_4 = StringLockDescriptor.of("lock4"); + private static final byte[] BYTES_OLD = PtBytes.toBytes("old"); + private static final byte[] BYTES_NEW = PtBytes.toBytes("new"); + private static final byte[] BYTES_DELETED = PtBytes.toBytes("deleted"); + private static final byte[] BYTES_CREATED = PtBytes.toBytes("created"); + private static final List LOCK_LIST = ImmutableList.of(LOCK_1, LOCK_2, LOCK_3, LOCK_4); + // Although this is quite verbose, we explicitly want to test all possible types of change metadata and ensure + // that we do the conversion right for each of them. + private static final LockRequestMetadata LOCK_REQUEST_METADATA = LockRequestMetadata.of(ImmutableMap.of( + LOCK_1, + ChangeMetadata.unchanged(), + LOCK_2, + ChangeMetadata.updated(BYTES_OLD, BYTES_NEW), + LOCK_3, + ChangeMetadata.deleted(BYTES_DELETED), + LOCK_4, + ChangeMetadata.created(BYTES_CREATED))); + private static final Map CONJURE_LOCKS_WITH_METADATA = ImmutableMap.of( + 0, + ConjureChangeMetadata.unchanged(ConjureUnchangedChangeMetadata.of()), + 1, + ConjureChangeMetadata.updated( + ConjureUpdatedChangeMetadata.of(Bytes.from(BYTES_OLD), Bytes.from(BYTES_NEW))), + 2, + ConjureChangeMetadata.deleted(ConjureDeletedChangeMetadata.of(Bytes.from(BYTES_DELETED))), + 3, + ConjureChangeMetadata.created(ConjureCreatedChangeMetadata.of(Bytes.from(BYTES_CREATED)))); + + private static ConjureMetadataConversionResult conjureMetadataConversionResult; + + // This is a good candidate for a static construction method, but we would rather avoid testing internals of + // IndexEncodingUtils (checksum computation) within the tests for Conjure conversion. + @BeforeClass + public static void setup() { + KeyListChecksum checksum = + IndexEncodingUtils.computeChecksum(ConjureLockRequestMetadataUtils.DEFAULT_CHECKSUM_TYPE, LOCK_LIST); + ConjureLockRequestMetadata conjureMetadata = ConjureLockRequestMetadata.builder() + .indexToChangeMetadata(CONJURE_LOCKS_WITH_METADATA) + .lockListChecksum( + ConjureLockDescriptorListChecksum.of(checksum.type().getId(), Bytes.from(checksum.value()))) + .build(); + conjureMetadataConversionResult = ConjureMetadataConversionResult.builder() + .lockList(LOCK_LIST) + .conjureMetadata(conjureMetadata) + .build(); + } + + @Test + public void convertsToConjureCorrectly() { + // ImmutableSet remembers insertion order, which is critical here + assertThat(ConjureLockRequestMetadataUtils.toConjureIndexEncoded( + ImmutableSet.copyOf(LOCK_LIST), LOCK_REQUEST_METADATA)) + .isEqualTo(conjureMetadataConversionResult); + } + + @Test + public void convertsFromConjureCorrectly() { + assertThat(ConjureLockRequestMetadataUtils.fromConjureIndexEncoded(conjureMetadataConversionResult)) + .isEqualTo(LOCK_REQUEST_METADATA); + } + + @Test + public void canConvertSparseMetadata() { + List lockDescriptors = IntStream.range(0, 10) + .mapToObj(Integer::toString) + .map(StringLockDescriptor::of) + .collect(Collectors.toList()); + // Unique metadata on some locks, but not all + Map lockDescriptorToChangeMetadata = ImmutableMap.of( + lockDescriptors.get(0), ChangeMetadata.created(BYTES_CREATED), + lockDescriptors.get(4), ChangeMetadata.deleted(BYTES_DELETED), + lockDescriptors.get(9), ChangeMetadata.unchanged(), + lockDescriptors.get(5), ChangeMetadata.updated(BYTES_OLD, BYTES_NEW), + lockDescriptors.get(7), ChangeMetadata.unchanged()); + LockRequestMetadata metadata = LockRequestMetadata.of(lockDescriptorToChangeMetadata); + + assertThat(ConjureLockRequestMetadataUtils.fromConjureIndexEncoded( + ConjureLockRequestMetadataUtils.toConjureIndexEncoded( + ImmutableSet.copyOf(lockDescriptors), metadata))) + .isEqualTo(metadata); + } + + @Test + public void changedLockOrderIsDetected() { + List modifiedLockList = new ArrayList<>(LOCK_LIST); + Collections.swap(modifiedLockList, 0, 1); + ConjureMetadataConversionResult conversionResult = ImmutableConjureMetadataConversionResult.copyOf( + conjureMetadataConversionResult) + .withLockList(modifiedLockList); + assertThatLoggableExceptionThrownBy( + () -> ConjureLockRequestMetadataUtils.fromConjureIndexEncoded(conversionResult)) + .isInstanceOf(SafeIllegalArgumentException.class) + .hasMessageStartingWith("Key list integrity check failed"); + } + + @Test + public void handlesEmptyData() { + assertThat(ConjureLockRequestMetadataUtils.fromConjureIndexEncoded( + ConjureLockRequestMetadataUtils.toConjureIndexEncoded( + ImmutableSet.of(), LockRequestMetadata.of(ImmutableMap.of())))) + .isEqualTo(LockRequestMetadata.of(ImmutableMap.of())); + } +} diff --git a/timelock-api/src/main/conjure/timelock-api.yml b/timelock-api/src/main/conjure/timelock-api.yml index 03226a0e6be..90aeb7cae10 100644 --- a/timelock-api/src/main/conjure/timelock-api.yml +++ b/timelock-api/src/main/conjure/timelock-api.yml @@ -93,6 +93,44 @@ types: inclusiveUpper: Long ConjureGetFreshTimestampsResponseV2: alias: ConjureTimestampRange + ConjureUnchangedChangeMetadata: + fields: {} + ConjureUpdatedChangeMetadata: + fields: + oldValue: + type: binary + safety: unsafe + newValue: + type: binary + safety: unsafe + ConjureDeletedChangeMetadata: + fields: + oldValue: + type: binary + safety: unsafe + ConjureCreatedChangeMetadata: + fields: + newValue: + type: binary + safety: unsafe + ConjureChangeMetadata: + union: + unchanged: ConjureUnchangedChangeMetadata + updated: ConjureUpdatedChangeMetadata + deleted: ConjureDeletedChangeMetadata + created: ConjureCreatedChangeMetadata + ConjureLockDescriptorListChecksum: + fields: + typeId: + type: integer + safety: safe + value: + type: binary + safety: unsafe + ConjureLockRequestMetadata: + fields: + indexToChangeMetadata: map + lockListChecksum: ConjureLockDescriptorListChecksum ConjureLockDescriptor: alias: binary safety: unsafe