Merge pull request #278 from bazsi/transport-remove-transport-registry

transport: get rid of the TransportFactoryId abstraction and the associated registry

lib/apphook.c

@@ -44,7 +44,6 @@
 #include "mainloop.h"
 #include "secret-storage/nondumpable-allocator.h"
 #include "secret-storage/secret-storage.h"
-#include "transport/transport-factory-id.h"
 #include "timeutils/timeutils.h"
 #include "msg-stats.h"
 #include "timeutils/cache.h"
@@ -239,7 +238,6 @@ app_startup(void)
   scratch_buffers_allocator_init();
   nondumpable_setlogger(nondumpable_allocator_msg_debug, nondumpable_allocator_msg_fatal);
   secret_storage_init();
-  transport_factory_id_global_init();
   scratch_buffers_global_init();
   msg_stats_init();
   timeutils_global_init();
@@ -295,7 +293,6 @@ app_shutdown(void)
   hostname_global_deinit();
   crypto_deinit();
   msg_deinit();
-  transport_factory_id_global_deinit();
 
 
   /* NOTE: the iv_deinit() call should come here, but there's some exit

lib/logproto/logproto-proxied-text-server.c

@@ -496,7 +496,7 @@ _fetch_into_proxy_buffer(LogProtoProxiedTextServer *self)
 static gboolean
 _switch_to_tls(LogProtoProxiedTextServer *self)
 {
-  if (!multitransport_switch((MultiTransport *)self->super.super.super.transport, transport_factory_tls_id()))
+  if (!multitransport_switch((MultiTransport *)self->super.super.super.transport, TRANSPORT_FACTORY_TLS_ID))
     {
       msg_error("proxied-tls failed to switch to TLS");
       return FALSE;

lib/transport/CMakeLists.txt

@@ -6,9 +6,7 @@ set(TRANSPORT_HEADERS
     transport/transport-pipe.h
     transport/transport-socket.h
     transport/transport-udp-socket.h
-    transport/transport-factory-id.h
     transport/transport-factory.h
-    transport/transport-factory-registry.h
     transport/multitransport.h
     transport/transport-factory-tls.h
     transport/transport-factory-socket.h
@@ -25,8 +23,6 @@ set(TRANSPORT_SOURCES
     transport/transport-socket.c
     transport/transport-udp-socket.c
     transport/transport-tls.c
-    transport/transport-factory-id.c
-    transport/transport-factory-registry.c
     transport/multitransport.c
     transport/transport-factory-tls.c
     transport/transport-factory-socket.c

lib/transport/Makefile.am

@@ -10,9 +10,7 @@ transportinclude_HEADERS = \
 	lib/transport/transport-pipe.h	\
 	lib/transport/transport-socket.h \
 	lib/transport/transport-udp-socket.h \
-	lib/transport/transport-factory-id.h \
 	lib/transport/transport-factory.h \
-	lib/transport/transport-factory-registry.h \
 	lib/transport/multitransport.h \
 	lib/transport/transport-factory-tls.h \
 	lib/transport/transport-factory-socket.h \
@@ -27,8 +25,6 @@ transport_sources = \
 	lib/transport/transport-pipe.c	\
 	lib/transport/transport-socket.c \
 	lib/transport/transport-udp-socket.c \
-	lib/transport/transport-factory-id.c \
-	lib/transport/transport-factory-registry.c \
 	lib/transport/multitransport.c \
 	lib/transport/transport-factory-tls.c \
 	lib/transport/transport-factory-socket.c \

lib/transport/multitransport.c

@@ -23,12 +23,12 @@
  */
 
 #include "transport/multitransport.h"
-#include "transport/transport-factory-registry.h"
 #include "messages.h"
 
-void multitransport_add_factory(MultiTransport *self, TransportFactory *transport_factory)
+void
+multitransport_add_factory(MultiTransport *self, TransportFactory *transport_factory)
 {
-  transport_factory_registry_add(self->registry, transport_factory);
+  g_hash_table_insert(self->registry, (gpointer) transport_factory->id, transport_factory);
 }
 
 static void
@@ -42,14 +42,14 @@ _do_transport_switch(MultiTransport *self, LogTransport *new_transport, const Tr
 }
 
 static const TransportFactory *
-_lookup_transport_factory(TransportFactoryRegistry *registry, const TransportFactoryId *factory_id)
+_lookup_transport_factory(MultiTransport *self, const gchar *factory_id)
 {
-  const TransportFactory *factory = transport_factory_registry_lookup(registry, factory_id);
+  const TransportFactory *factory = g_hash_table_lookup(self->registry, factory_id);
 
   if (!factory)
     {
       msg_error("Requested transport not found",
-                evt_tag_str("transport", transport_factory_id_get_transport_name(factory_id)));
+                evt_tag_str("transport", factory_id));
       return NULL;
     }
 
@@ -60,25 +60,25 @@ static LogTransport *
 _construct_transport(const TransportFactory *factory, gint fd)
 {
   LogTransport *transport = transport_factory_construct_transport(factory, fd);
-  const TransportFactoryId *factory_id = transport_factory_get_id(factory);
 
   if (!transport)
     {
       msg_error("Failed to construct transport",
-                evt_tag_str("transport", transport_factory_id_get_transport_name(factory_id)));
+                evt_tag_str("transport", factory->id));
       return NULL;
     }
 
   return transport;
 }
 
-gboolean multitransport_switch(MultiTransport *self, const TransportFactoryId *factory_id)
+gboolean
+multitransport_switch(MultiTransport *self, const gchar *factory_id)
 {
   msg_debug("Transport switch requested",
             evt_tag_str("active-transport", self->active_transport->name),
-            evt_tag_str("requested-transport", transport_factory_id_get_transport_name(factory_id)));
+            evt_tag_str("requested-transport", factory_id));
 
-  const TransportFactory *transport_factory = _lookup_transport_factory(self->registry, factory_id);
+  const TransportFactory *transport_factory = _lookup_transport_factory(self, factory_id);
   if (!transport_factory)
     return FALSE;
 
@@ -95,9 +95,9 @@ gboolean multitransport_switch(MultiTransport *self, const TransportFactoryId *f
 }
 
 gboolean
-multitransport_contains_factory(MultiTransport *self, const TransportFactoryId *factory_id)
+multitransport_contains_factory(MultiTransport *self, const gchar *factory_id)
 {
-  const TransportFactory *factory = transport_factory_registry_lookup(self->registry, factory_id);
+  const TransportFactory *factory = g_hash_table_lookup(self->registry, factory_id);
 
   return (factory != NULL);
 }
@@ -128,21 +128,20 @@ _multitransport_free(LogTransport *s)
   MultiTransport *self = (MultiTransport *)s;
   s->fd = log_transport_release_fd(self->active_transport);
   log_transport_free(self->active_transport);
-  transport_factory_registry_free(self->registry);
+  g_hash_table_unref(self->registry);
   log_transport_free_method(s);
 }
 
 LogTransport *
 multitransport_new(TransportFactory *default_transport_factory, gint fd)
 {
   MultiTransport *self = g_new0(MultiTransport, 1);
-  self->registry = transport_factory_registry_new();
-  transport_factory_registry_add(self->registry, default_transport_factory);
 
   log_transport_init_instance(&self->super, fd);
   self->super.read = _multitransport_read;
   self->super.write = _multitransport_write;
   self->super.free_fn = _multitransport_free;
+  self->registry = g_hash_table_new_full(g_str_hash, g_str_equal, NULL, (GDestroyNotify) transport_factory_free);
   self->active_transport = transport_factory_construct_transport(default_transport_factory, fd);
   self->active_transport_factory = default_transport_factory;
 

lib/transport/multitransport.h

@@ -27,22 +27,21 @@
 
 #include "transport/logtransport.h"
 #include "transport/transport-factory.h"
-#include "transport/transport-factory-registry.h"
 
 typedef struct _MultiTransport MultiTransport;
 
 struct _MultiTransport
 {
   LogTransport super;
-  TransportFactoryRegistry *registry;
+  GHashTable *registry;
   LogTransport *active_transport;
   const TransportFactory *active_transport_factory;
 };
 
 LogTransport *multitransport_new(TransportFactory *default_transport_factory, gint fd);
-void multitransport_add_factory(MultiTransport *, TransportFactory *);
-gboolean multitransport_switch(MultiTransport *, const TransportFactoryId *);
-gboolean multitransport_contains_factory(MultiTransport *, const TransportFactoryId *);
+void multitransport_add_factory(MultiTransport *self, TransportFactory *);
+gboolean multitransport_switch(MultiTransport *self, const gchar *id);
+gboolean multitransport_contains_factory(MultiTransport *self, const gchar *id);
 
 #endif
 

lib/transport/tests/CMakeLists.txt

@@ -1,5 +1,3 @@
 add_unit_test(CRITERION TARGET test_aux_data)
-add_unit_test(CRITERION TARGET test_transport_factory_id)
 add_unit_test(CRITERION TARGET test_transport_factory)
-add_unit_test(CRITERION TARGET test_transport_factory_registry)
 add_unit_test(CRITERION TARGET test_multitransport)

lib/transport/tests/Makefile.am

@@ -1,8 +1,6 @@
 lib_transport_tests_TESTS		 = \
 	lib/transport/tests/test_aux_data \
-	lib/transport/tests/test_transport_factory_id \
 	lib/transport/tests/test_transport_factory \
-	lib/transport/tests/test_transport_factory_registry \
 	lib/transport/tests/test_multitransport
 
 EXTRA_DIST += lib/transport/tests/CMakeLists.txt
@@ -15,24 +13,12 @@ lib_transport_tests_test_aux_data_LDADD	 = $(TEST_LDADD)
 lib_transport_tests_test_aux_data_SOURCES = 			\
 	lib/transport/tests/test_aux_data.c
 
-lib_transport_tests_test_transport_factory_id_CFLAGS  = $(TEST_CFLAGS) \
-	-I${top_srcdir}/lib/transport/tests
-lib_transport_tests_test_transport_factory_id_LDADD	 = $(TEST_LDADD)
-lib_transport_tests_test_transport_factory_id_SOURCES = 			\
-	lib/transport/tests/test_transport_factory_id.c
-
 lib_transport_tests_test_transport_factory_CFLAGS  = $(TEST_CFLAGS) \
 	-I${top_srcdir}/lib/transport/tests
 lib_transport_tests_test_transport_factory_LDADD	 = $(TEST_LDADD)
 lib_transport_tests_test_transport_factory_SOURCES = 			\
 	lib/transport/tests/test_transport_factory.c
 
-lib_transport_tests_test_transport_factory_registry_CFLAGS  = $(TEST_CFLAGS) \
-	-I${top_srcdir}/lib/transport/tests
-lib_transport_tests_test_transport_factory_registry_LDADD	 = $(TEST_LDADD)
-lib_transport_tests_test_transport_factory_registry_SOURCES = 			\
-	lib/transport/tests/test_transport_factory_registry.c
-
 lib_transport_tests_test_multitransport_CFLAGS  = $(TEST_CFLAGS) \
 	-I${top_srcdir}/lib/transport/tests
 lib_transport_tests_test_multitransport_LDADD	 = $(TEST_LDADD)

lib/transport/tests/test_multitransport.c

@@ -56,7 +56,6 @@
   { \
     TransportFactory super; \
   }; \
-  DEFINE_TRANSPORT_FACTORY_ID_FUN(#FunPrefix, FunPrefix ## _transport_factory_id); \
   static LogTransport * \
   FunPrefix ## _factory_construct(const TransportFactory *s, gint fd) \
   {\
@@ -68,7 +67,7 @@
   FunPrefix ## _transport_factory_new(void) \
   { \
     TypePrefix ## TransportFactory *self = g_new0(TypePrefix ## TransportFactory, 1); \
-    self->super.id = FunPrefix ## _transport_factory_id(); \
+    self->super.id = #FunPrefix; \
     self->super.construct_transport = FunPrefix ## _factory_construct; \
     return &self->super; \
   } \
@@ -92,16 +91,15 @@ Test(multitransport, test_switch_transport)
   cr_expect_str_eq(multi_transport->active_transport->name, "default");
 
   multitransport_add_factory(multi_transport, fake_factory);
-  cr_expect_not(multitransport_contains_factory(multi_transport, unregistered_transport_factory_id()));
-  cr_expect(multitransport_contains_factory(multi_transport, fake_transport_factory_id()));
-  cr_expect(multitransport_contains_factory(multi_transport, default_transport_factory_id()));
-  cr_expect_not(multitransport_switch(multi_transport, unregistered_transport_factory_id()));
+  cr_expect_not(multitransport_contains_factory(multi_transport, "unregistered"));
+  cr_expect(multitransport_contains_factory(multi_transport, "fake"));
+  cr_expect_not(multitransport_switch(multi_transport, "unregistered"));
   cr_expect_eq(multi_transport->active_transport->read, default_read);
   cr_expect_eq(multi_transport->active_transport->write, default_write);
   cr_expect_eq(multi_transport->active_transport_factory, default_factory);
   cr_expect_str_eq(multi_transport->active_transport->name, "default");
 
-  cr_expect(multitransport_switch(multi_transport, fake_transport_factory_id()));
+  cr_expect(multitransport_switch(multi_transport, "fake"));
   cr_expect_eq(multi_transport->active_transport->read, fake_read);
   cr_expect_eq(multi_transport->active_transport->write, fake_write);
   cr_expect_eq(multi_transport->active_transport_factory, fake_factory);

lib/transport/tests/test_transport_factory.c

@@ -69,8 +69,6 @@ struct _FakeTransportFactory
   TransportFactory super;
 };
 
-DEFINE_TRANSPORT_FACTORY_ID_FUN("fake", _fake_transport_factory_id);
-
 static LogTransport *
 _transport_factory_construct(const TransportFactory *s, gint fd)
 {
@@ -85,7 +83,7 @@ TransportFactory *
 _fake_transport_factory_new(void)
 {
   FakeTransportFactory *instance = g_new0(FakeTransportFactory, 1);
-  instance->super.id = _fake_transport_factory_id();
+  instance->super.id = "fake";
   instance->super.construct_transport = _transport_factory_construct;
   return &instance->super;
 }