From 13d361d017a4e96331ced0962c18e2c4b6b18bf5 Mon Sep 17 00:00:00 2001 From: Josef Ezra Date: Tue, 12 Oct 2021 19:53:40 -0400 Subject: [PATCH] Sonar cloud "Cognitive Complexity" warnings are now enforced. Had to split some functions. This seems to be a complicated way to reduce complexity. Most methods are, and should be, a "single task" leaf, but some are skeletons/trunks - and implementing a certain "storyline" agorithm. Spreading this algorithm among multiple functions makes it anything but more readable. --- .../provider/foundationdb/IndexingBase.java | 74 +++++++++++-------- .../provider/foundationdb/OnlineIndexer.java | 12 ++- 2 files changed, 53 insertions(+), 33 deletions(-) diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/IndexingBase.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/IndexingBase.java index e96ae05e70..e940a3e9b2 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/IndexingBase.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/IndexingBase.java @@ -389,26 +389,7 @@ private CompletableFuture setIndexingTypeOrThrow(FDBRecordStore store, boo IndexBuildProto.IndexBuildIndexingStamp.Method.BY_RECORDS) { // backward compatibility - maybe continuing an old BY_RECORD session return isWriteOnlyButNoRecordScanned(store, index) - .thenCompose(noRecordScanned -> { - if (noRecordScanned) { - // an empty type stamp, and nothing was indexed - it is safe to write stamp - if (LOGGER.isInfoEnabled()) { - LOGGER.info(KeyValueLogMessage.build("no scanned ranges - continue indexing") - .addKeysAndValues(common.indexLogMessageKeyValues()) - .toString()); - } - transaction.set(stampKey, indexingTypeStamp.toByteArray()); - return AsyncUtil.DONE; - } - // Here: there is no type stamp, but indexing is ongoing. For backward compatibility reasons, we'll consider it a BY_RECORDS stamp - if (LOGGER.isInfoEnabled()) { - LOGGER.info(KeyValueLogMessage.build("continuation with null type stamp, assuming previous by-records scan") - .addKeysAndValues(common.indexLogMessageKeyValues()) - .toString()); - } - final IndexBuildProto.IndexBuildIndexingStamp fakeSavedStamp = IndexingByRecords.compileIndexingTypeStamp(); - throw newPartlyBuildException(true, fakeSavedStamp, indexingTypeStamp, index); - }); + .thenCompose(noRecordScanned -> throwAsByRecordsUnlessNoRecordWasScanned(noRecordScanned, transaction, index, stampKey, indexingTypeStamp)); } // Here: either not a continuedBuild (new session), or a BY_RECORD session (allowed to overwrite the null stamp) transaction.set(stampKey, indexingTypeStamp.toByteArray()); @@ -430,21 +411,56 @@ private CompletableFuture setIndexingTypeOrThrow(FDBRecordStore store, boo if (forceStampOverwrite) { // and a continued Build // check if partly built return isWriteOnlyButNoRecordScanned(store, index) - .thenCompose(noRecordScanned -> { - if (noRecordScanned) { - // we can safely overwrite the previous type stamp - transaction.set(stampKey, indexingTypeStamp.toByteArray()); - return AsyncUtil.DONE; - } - // A force overwrite cannot be allowed when partly built - throw newPartlyBuildException(continuedBuild, savedStamp, indexingTypeStamp, index); - }); + .thenCompose(noRecordScanned -> + throwUnlessNoRecordWasScanned(noRecordScanned, transaction, index, stampKey, indexingTypeStamp, + savedStamp, continuedBuild)); } // fall down to exception throw newPartlyBuildException(continuedBuild, savedStamp, indexingTypeStamp, index); }); } + @Nonnull + private CompletableFuture throwAsByRecordsUnlessNoRecordWasScanned(boolean noRecordScanned, Transaction transaction, + Index index, byte[] stampKey, + IndexBuildProto.IndexBuildIndexingStamp indexingTypeStamp) { + // A complicated way to reduce complexity. + if (noRecordScanned) { + // an empty type stamp, and nothing was indexed - it is safe to write stamp + if (LOGGER.isInfoEnabled()) { + LOGGER.info(KeyValueLogMessage.build("no scanned ranges - continue indexing") + .addKeysAndValues(common.indexLogMessageKeyValues()) + .toString()); + } + transaction.set(stampKey, indexingTypeStamp.toByteArray()); + return AsyncUtil.DONE; + } + // Here: there is no type stamp, but indexing is ongoing. For backward compatibility reasons, we'll consider it a BY_RECORDS stamp + if (LOGGER.isInfoEnabled()) { + LOGGER.info(KeyValueLogMessage.build("continuation with null type stamp, assuming previous by-records scan") + .addKeysAndValues(common.indexLogMessageKeyValues()) + .toString()); + } + final IndexBuildProto.IndexBuildIndexingStamp fakeSavedStamp = IndexingByRecords.compileIndexingTypeStamp(); + throw newPartlyBuildException(true, fakeSavedStamp, indexingTypeStamp, index); + } + + @Nonnull + private CompletableFuture throwUnlessNoRecordWasScanned(boolean noRecordScanned, Transaction transaction, + Index index, byte[] stampKey, + IndexBuildProto.IndexBuildIndexingStamp indexingTypeStamp, + IndexBuildProto.IndexBuildIndexingStamp savedStamp, + boolean continuedBuild) { + // Ditto (a complicated way to reduce complexity) + if (noRecordScanned) { + // we can safely overwrite the previous type stamp + transaction.set(stampKey, indexingTypeStamp.toByteArray()); + return AsyncUtil.DONE; + } + // A force overwrite cannot be allowed when partly built + throw newPartlyBuildException(continuedBuild, savedStamp, indexingTypeStamp, index); + } + @Nonnull private CompletableFuture setScrubberTypeOrThrow(FDBRecordStore store) { // HERE: The index must be readable, checked by the caller diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/OnlineIndexer.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/OnlineIndexer.java index c6e3e5ea59..b2ff3babaf 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/OnlineIndexer.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/OnlineIndexer.java @@ -1841,13 +1841,14 @@ private void determineIndexingPolicy() { } private void validate() { - validateIndexesAndTypes(); + final RecordMetaData metaData = getRecordMetaData(); + validateIndexes(metaData); + validateTypes(metaData); validateLimits(); } - private void validateIndexesAndTypes() { - final RecordMetaData metaData = getRecordMetaData(); - if (this.targetIndexes == null || this.targetIndexes.isEmpty()) { + private void validateIndexes(RecordMetaData metaData) { + if (this.targetIndexes.isEmpty()) { throw new MetaDataException("index must be set"); } // Remove duplications (if any) @@ -1867,6 +1868,9 @@ private void validateIndexesAndTypes() { throw new MetaDataException("Index " + index.getName() + " not contained within specified metadata"); } } + } + + private void validateTypes(RecordMetaData metaData) { if (recordTypes != null) { for (RecordType recordType : recordTypes) { if (recordType != metaData.getIndexableRecordType(recordType.getName())) {