Skip to content

Commit

Permalink
apacheGH-39339: [C++] Add ForceCachedHierarchicalNamespaceSupport to …
Browse files Browse the repository at this point in the history
…help with testing (apache#39340)

### Rationale for this change

This ensures all the branches in the `AzureFileSystem` code operations are tested.
For instance, many operations executed on a missing container, wouldn't
get a `HNSSupport::kContainerNotFound` error if the cached `HNSSupport` was
already known due to a previous operation that cached the `HNSSupport` value.

### What changes are included in this PR?

Introduction of the helper that overrides `cached_hns_support_` and enumeration of the scenarios.

### Are these changes tested?

Yes. This is a test improvement PR.

* Closes: apache#39339

Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
  • Loading branch information
felipecrv authored and dgreiss committed Feb 17, 2024
1 parent 8f01956 commit f94d76c
Show file tree
Hide file tree
Showing 3 changed files with 291 additions and 203 deletions.
36 changes: 32 additions & 4 deletions cpp/src/arrow/filesystem/azurefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -941,14 +941,38 @@ class AzureFileSystem::Impl {
break;
}
ARROW_ASSIGN_OR_RAISE(
cached_hns_support_,
auto hns_support,
internal::CheckIfHierarchicalNamespaceIsEnabled(adlfs_client, options_));
DCHECK_NE(cached_hns_support_, HNSSupport::kUnknown);
// Caller should handle kContainerNotFound case appropriately.
return cached_hns_support_;
DCHECK_NE(hns_support, HNSSupport::kUnknown);
if (hns_support == HNSSupport::kContainerNotFound) {
// Caller should handle kContainerNotFound case appropriately as it knows the
// container this refers to, but the cached value in that case should remain
// kUnknown before we get a CheckIfHierarchicalNamespaceIsEnabled result that
// is not kContainerNotFound.
cached_hns_support_ = HNSSupport::kUnknown;
} else {
cached_hns_support_ = hns_support;
}
return hns_support;
}

public:
/// This is used from unit tests to ensure we perform operations on all the
/// possible states of cached_hns_support_.
void ForceCachedHierarchicalNamespaceSupport(int support) {
auto hns_support = static_cast<HNSSupport>(support);
switch (hns_support) {
case HNSSupport::kUnknown:
case HNSSupport::kContainerNotFound:
case HNSSupport::kDisabled:
case HNSSupport::kEnabled:
cached_hns_support_ = hns_support;
return;
}
// This is reachable if an invalid int is cast to enum class HNSSupport.
DCHECK(false) << "Invalid enum HierarchicalNamespaceSupport value.";
}

Result<FileInfo> GetFileInfo(const AzureLocation& location) {
if (location.container.empty()) {
DCHECK(location.path.empty());
Expand Down Expand Up @@ -1560,6 +1584,10 @@ AzureFileSystem::AzureFileSystem(std::unique_ptr<Impl>&& impl)
default_async_is_sync_ = false;
}

void AzureFileSystem::ForceCachedHierarchicalNamespaceSupport(int hns_support) {
impl_->ForceCachedHierarchicalNamespaceSupport(hns_support);
}

Result<std::shared_ptr<AzureFileSystem>> AzureFileSystem::Make(
const AzureOptions& options, const io::IOContext& io_context) {
ARROW_ASSIGN_OR_RAISE(auto impl, AzureFileSystem::Impl::Make(options, io_context));
Expand Down
5 changes: 5 additions & 0 deletions cpp/src/arrow/filesystem/azurefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ class DataLakeServiceClient;

namespace arrow::fs {

class TestAzureFileSystem;

/// Options for the AzureFileSystem implementation.
struct ARROW_EXPORT AzureOptions {
/// \brief hostname[:port] of the Azure Blob Storage Service.
Expand Down Expand Up @@ -156,6 +158,9 @@ class ARROW_EXPORT AzureFileSystem : public FileSystem {

explicit AzureFileSystem(std::unique_ptr<Impl>&& impl);

friend class TestAzureFileSystem;
void ForceCachedHierarchicalNamespaceSupport(int hns_support);

public:
~AzureFileSystem() override = default;

Expand Down
Loading

0 comments on commit f94d76c

Please sign in to comment.