From 60d57c5c43373973ea1159044648596f1d58b4c0 Mon Sep 17 00:00:00 2001 From: alytantawyy Date: Mon, 9 Nov 2026 11:23:49 -0800 Subject: [PATCH 1/2] GH-51816: [GLib] Add filesystem GType registration mechanism --- c_glib/arrow-glib/file-system.cpp | 102 ++++++++++++++++++++---- c_glib/arrow-glib/file-system.h | 4 + c_glib/meson.build | 1 + c_glib/test/meson.build | 23 ++++++ c_glib/test/test-file-system.cpp | 125 ++++++++++++++++++++++++++++++ c_glib/test/test-file-system.rb | 50 ++++++++++++ ci/scripts/c_glib_test.sh | 1 + 7 files changed, 291 insertions(+), 15 deletions(-) create mode 100644 c_glib/test/meson.build create mode 100644 c_glib/test/test-file-system.cpp create mode 100644 c_glib/test/test-file-system.rb diff --git a/c_glib/arrow-glib/file-system.cpp b/c_glib/arrow-glib/file-system.cpp index 9ba494e40595..1b92c1e50b04 100644 --- a/c_glib/arrow-glib/file-system.cpp +++ b/c_glib/arrow-glib/file-system.cpp @@ -17,6 +17,10 @@ * under the License. */ +#include +#include +#include + #include #include @@ -545,6 +549,26 @@ garrow_file_selector_class_init(GArrowFileSelectorClass *klass) /* arrow::fs::FileSystem */ +struct GArrowFileSystemTypeRegistry +{ + std::mutex mutex; + std::unordered_map types = { + {"local", GARROW_TYPE_LOCAL_FILE_SYSTEM}, + {"hdfs", GARROW_TYPE_HDFS_FILE_SYSTEM}, + {"s3", GARROW_TYPE_S3_FILE_SYSTEM}, + {"gcs", GARROW_TYPE_GCS_FILE_SYSTEM}, + {"abfs", GARROW_TYPE_AZURE_FILE_SYSTEM}, + {"mock", GARROW_TYPE_MOCK_FILE_SYSTEM}, + }; +}; + +static GArrowFileSystemTypeRegistry & +garrow_file_system_get_type_registry(void) +{ + static GArrowFileSystemTypeRegistry registry; + return registry; +} + typedef struct GArrowFileSystemPrivate_ { std::shared_ptr file_system; @@ -554,7 +578,7 @@ enum { PROP_FILE_SYSTEM = 1 }; -G_DEFINE_ABSTRACT_TYPE_WITH_PRIVATE(GArrowFileSystem, garrow_file_system, G_TYPE_OBJECT) +G_DEFINE_TYPE_WITH_PRIVATE(GArrowFileSystem, garrow_file_system, G_TYPE_OBJECT) #define GARROW_FILE_SYSTEM_GET_PRIVATE(obj) \ static_cast( \ @@ -614,6 +638,58 @@ garrow_file_system_class_init(GArrowFileSystemClass *klass) g_object_class_install_property(gobject_class, PROP_FILE_SYSTEM, spec); } +/** + * garrow_file_system_register_type: + * @type_name: A non-empty Arrow C++ file system type name. + * @gtype: A non-abstract #GArrowFileSystem type. + * @error: (nullable): Return location for a #GError or %NULL. + * + * Registers @gtype as the GLib wrapper type for @type_name. The type name + * is copied and the registration lasts for the lifetime of the process. + * Built-in file system wrapper types are registered automatically. + * Registering a name that is already registered is an error. + * + * This function is thread-safe. + * + * Returns: %TRUE on success, %FALSE on error. + * + * Since: 27.0.0 + */ +gboolean +garrow_file_system_register_type(const gchar *type_name, GType gtype, GError **error) +{ + const gchar *context = "[file-system][register-type]"; + if (!type_name || !*type_name) { + g_set_error(error, + GARROW_ERROR, + GARROW_ERROR_INVALID, + "%s type name must not be NULL or empty", + context); + return FALSE; + } + if (!g_type_is_a(gtype, GARROW_TYPE_FILE_SYSTEM) || G_TYPE_IS_ABSTRACT(gtype)) { + g_set_error(error, + GARROW_ERROR, + GARROW_ERROR_INVALID, + "%s GType must be a non-abstract GArrowFileSystem type", + context); + return FALSE; + } + + auto ®istry = garrow_file_system_get_type_registry(); + std::lock_guard lock(registry.mutex); + if (!registry.types.emplace(type_name, gtype).second) { + g_set_error(error, + GARROW_ERROR, + GARROW_ERROR_KEY, + "%s file system type is already registered: %s", + context, + type_name); + return FALSE; + } + return TRUE; +} + /** * garrow_file_system_create: * @uri: An URI to specify file system with options. If you only have an @@ -623,8 +699,8 @@ garrow_file_system_class_init(GArrowFileSystemClass *klass) * This is a factory function to create a specific #GArrowFileSystem * object. * - * Returns: (nullable) (transfer full): The newly created file system - * that is an object of a subclass of #GArrowFileSystem. + * Returns: (nullable) (transfer full): The newly created #GArrowFileSystem + * or an object of one of its subclasses. * * Since: 3.0.0 */ @@ -1598,20 +1674,16 @@ garrow_file_system_new_raw(std::shared_ptr *arrow_file_sy const auto &type_name = (*arrow_file_system)->type_name(); GType file_system_type = GARROW_TYPE_FILE_SYSTEM; - if (type_name == "local") { - file_system_type = GARROW_TYPE_LOCAL_FILE_SYSTEM; - } else if (type_name == "hdfs") { - file_system_type = GARROW_TYPE_HDFS_FILE_SYSTEM; - } else if (type_name == "s3") { - file_system_type = GARROW_TYPE_S3_FILE_SYSTEM; - } else if (type_name == "gcs") { - file_system_type = GARROW_TYPE_GCS_FILE_SYSTEM; - } else if (type_name == "abfs") { - file_system_type = GARROW_TYPE_AZURE_FILE_SYSTEM; - } else if (type_name == "mock") { - file_system_type = GARROW_TYPE_MOCK_FILE_SYSTEM; + auto ®istry = garrow_file_system_get_type_registry(); + { + std::lock_guard lock(registry.mutex); + auto it = registry.types.find(type_name); + if (it != registry.types.end()) { + file_system_type = it->second; + } } + // Wrapper class initialization may register additional types. return GARROW_FILE_SYSTEM( g_object_new(file_system_type, "file-system", arrow_file_system, NULL)); } diff --git a/c_glib/arrow-glib/file-system.h b/c_glib/arrow-glib/file-system.h index 9a903c6af68c..666271956df1 100644 --- a/c_glib/arrow-glib/file-system.h +++ b/c_glib/arrow-glib/file-system.h @@ -100,6 +100,10 @@ struct _GArrowFileSystemClass GObjectClass parent_class; }; +GARROW_AVAILABLE_IN_27_0 +gboolean +garrow_file_system_register_type(const gchar *type_name, GType gtype, GError **error); + GARROW_AVAILABLE_IN_3_0 GArrowFileSystem * garrow_file_system_create(const gchar *uri, GError **error); diff --git a/c_glib/meson.build b/c_glib/meson.build index 365066e932e8..d534d74f1a7f 100644 --- a/c_glib/meson.build +++ b/c_glib/meson.build @@ -313,6 +313,7 @@ if parquet.found() subdir('parquet-glib') endif subdir('example') +subdir('test') if get_option('doc') subdir('doc') diff --git a/c_glib/test/meson.build b/c_glib/test/meson.build new file mode 100644 index 000000000000..20ba13ada95e --- /dev/null +++ b/c_glib/test/meson.build @@ -0,0 +1,23 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. + +file_system_test = executable( + 'file-system-test', + 'test-file-system.cpp', + dependencies: arrow_glib, +) +test('filesystem registry', file_system_test) diff --git a/c_glib/test/test-file-system.cpp b/c_glib/test/test-file-system.cpp new file mode 100644 index 000000000000..564c925b8ff4 --- /dev/null +++ b/c_glib/test/test-file-system.cpp @@ -0,0 +1,125 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ + +#include +#include +#include + +#include + +#include +#include +#include + +namespace { + class TestFileSystem : public arrow::fs::LocalFileSystem { + public: + explicit TestFileSystem(std::string type_name) : type_name_(std::move(type_name)) {} + + std::string + type_name() const override + { + return type_name_; + } + + private: + std::string type_name_; + }; + + void + test_builtin() + { + std::shared_ptr file_system = + std::make_shared(); + g_autoptr(GArrowFileSystem) wrapper = garrow_file_system_new_raw(&file_system); + g_assert_true(GARROW_IS_LOCAL_FILE_SYSTEM(wrapper)); + g_assert_true(garrow_file_system_get_raw(wrapper) == file_system); + } + + void + test_registered() + { + auto wrapper_type = g_type_register_static_simple(GARROW_TYPE_FILE_SYSTEM, + "TestRegisteredFileSystem", + sizeof(GArrowFileSystemClass), + nullptr, + sizeof(GArrowFileSystem), + nullptr, + static_cast(0)); + std::string type_name = "test-registered"; + g_autoptr(GError) error = nullptr; + g_assert_true( + garrow_file_system_register_type(type_name.c_str(), wrapper_type, &error)); + g_assert_no_error(error); + type_name = "changed-after-registration"; + + std::shared_ptr file_system = + std::make_shared("test-registered"); + g_autoptr(GArrowFileSystem) wrapper = garrow_file_system_new_raw(&file_system); + g_assert_true(G_OBJECT_TYPE(wrapper) == wrapper_type); + g_assert_true(garrow_file_system_get_raw(wrapper) == file_system); + } + + void + test_fallback() + { + std::shared_ptr file_system = + std::make_shared("test-unregistered"); + g_autoptr(GArrowFileSystem) wrapper = garrow_file_system_new_raw(&file_system); + g_assert_true(G_OBJECT_TYPE(wrapper) == GARROW_TYPE_FILE_SYSTEM); + g_assert_true(garrow_file_system_get_raw(wrapper) == file_system); + + g_autofree gchar *type_name = garrow_file_system_get_type_name(wrapper); + g_assert_cmpstr(type_name, ==, "test-unregistered"); + + g_autoptr(GError) error = nullptr; + g_autoptr(GArrowFileInfo) info = + garrow_file_system_get_file_info(wrapper, g_get_tmp_dir(), &error); + g_assert_no_error(error); + g_assert_nonnull(info); + g_assert_true(garrow_file_info_is_dir(info)); + } + + void + test_abstract_type() + { + auto wrapper_type = g_type_register_static_simple(GARROW_TYPE_FILE_SYSTEM, + "TestAbstractFileSystem", + sizeof(GArrowFileSystemClass), + nullptr, + sizeof(GArrowFileSystem), + nullptr, + G_TYPE_FLAG_ABSTRACT); + g_autoptr(GError) error = nullptr; + g_assert_false( + garrow_file_system_register_type("test-abstract", wrapper_type, &error)); + g_assert_error(error, GARROW_ERROR, GARROW_ERROR_INVALID); + } +} // namespace + +int +main(int argc, char **argv) +{ + g_test_init(&argc, &argv, nullptr); + g_test_add_func("/file-system/builtin", test_builtin); + g_test_add_func("/file-system/registered", test_registered); + g_test_add_func("/file-system/fallback", test_fallback); + g_test_add_func("/file-system/abstract-type", test_abstract_type); + return g_test_run(); +} diff --git a/c_glib/test/test-file-system.rb b/c_glib/test/test-file-system.rb new file mode 100644 index 000000000000..4f942f95b632 --- /dev/null +++ b/c_glib/test/test-file-system.rb @@ -0,0 +1,50 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. + +class TestFileSystem < Test::Unit::TestCase + sub_test_case("register_type") do + def test_valid + assert_true(Arrow::FileSystem.register_type("test-file-system-valid", + Arrow::MockFileSystem.gtype)) + end + + def test_duplicate + type_name = "test-file-system-duplicate" + Arrow::FileSystem.register_type(type_name, Arrow::MockFileSystem.gtype) + assert_raise(Arrow::Error::Key) do + Arrow::FileSystem.register_type(type_name, Arrow::LocalFileSystem.gtype) + end + end + + def test_empty_name + assert_raise(Arrow::Error::Invalid) do + Arrow::FileSystem.register_type("", Arrow::MockFileSystem.gtype) + end + end + + def test_unrelated_type + assert_raise(Arrow::Error::Invalid) do + Arrow::FileSystem.register_type("test-file-system-unrelated", + Arrow::Int32Array.gtype) + end + end + end + + def test_create_mock + assert_instance_of(Arrow::MockFileSystem, Arrow::FileSystem.create("mock:///")) + end +end diff --git a/ci/scripts/c_glib_test.sh b/ci/scripts/c_glib_test.sh index bf385fe8dbd9..d30a030af769 100755 --- a/ci/scripts/c_glib_test.sh +++ b/ci/scripts/c_glib_test.sh @@ -65,6 +65,7 @@ fi popd pushd "${build_dir}" +test/file-system-test example/build example/extension-type if [ "${ARROW_GLIB_VAPI}" = "true" ]; then From 92dc3518a1f97ecd6c4aea21c3b5453ab50a6a76 Mon Sep 17 00:00:00 2001 From: alytantawyy Date: Mon, 9 Nov 2026 20:27:00 -0800 Subject: [PATCH 2/2] GH-51816: [GLib] Make filesystem registry tests portable to MSVC --- c_glib/test/test-file-system.cpp | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/c_glib/test/test-file-system.cpp b/c_glib/test/test-file-system.cpp index 564c925b8ff4..aba1acf7d977 100644 --- a/c_glib/test/test-file-system.cpp +++ b/c_glib/test/test-file-system.cpp @@ -47,9 +47,10 @@ namespace { { std::shared_ptr file_system = std::make_shared(); - g_autoptr(GArrowFileSystem) wrapper = garrow_file_system_new_raw(&file_system); + auto wrapper = garrow_file_system_new_raw(&file_system); g_assert_true(GARROW_IS_LOCAL_FILE_SYSTEM(wrapper)); g_assert_true(garrow_file_system_get_raw(wrapper) == file_system); + g_object_unref(wrapper); } void @@ -63,7 +64,7 @@ namespace { nullptr, static_cast(0)); std::string type_name = "test-registered"; - g_autoptr(GError) error = nullptr; + GError *error = nullptr; g_assert_true( garrow_file_system_register_type(type_name.c_str(), wrapper_type, &error)); g_assert_no_error(error); @@ -71,9 +72,10 @@ namespace { std::shared_ptr file_system = std::make_shared("test-registered"); - g_autoptr(GArrowFileSystem) wrapper = garrow_file_system_new_raw(&file_system); + auto wrapper = garrow_file_system_new_raw(&file_system); g_assert_true(G_OBJECT_TYPE(wrapper) == wrapper_type); g_assert_true(garrow_file_system_get_raw(wrapper) == file_system); + g_object_unref(wrapper); } void @@ -81,19 +83,21 @@ namespace { { std::shared_ptr file_system = std::make_shared("test-unregistered"); - g_autoptr(GArrowFileSystem) wrapper = garrow_file_system_new_raw(&file_system); + auto wrapper = garrow_file_system_new_raw(&file_system); g_assert_true(G_OBJECT_TYPE(wrapper) == GARROW_TYPE_FILE_SYSTEM); g_assert_true(garrow_file_system_get_raw(wrapper) == file_system); - g_autofree gchar *type_name = garrow_file_system_get_type_name(wrapper); + auto type_name = garrow_file_system_get_type_name(wrapper); g_assert_cmpstr(type_name, ==, "test-unregistered"); + g_free(type_name); - g_autoptr(GError) error = nullptr; - g_autoptr(GArrowFileInfo) info = - garrow_file_system_get_file_info(wrapper, g_get_tmp_dir(), &error); + GError *error = nullptr; + auto info = garrow_file_system_get_file_info(wrapper, g_get_tmp_dir(), &error); g_assert_no_error(error); g_assert_nonnull(info); g_assert_true(garrow_file_info_is_dir(info)); + g_object_unref(info); + g_object_unref(wrapper); } void @@ -106,10 +110,11 @@ namespace { sizeof(GArrowFileSystem), nullptr, G_TYPE_FLAG_ABSTRACT); - g_autoptr(GError) error = nullptr; + GError *error = nullptr; g_assert_false( garrow_file_system_register_type("test-abstract", wrapper_type, &error)); g_assert_error(error, GARROW_ERROR, GARROW_ERROR_INVALID); + g_clear_error(&error); } } // namespace