fix tests and clean up a few parts of the code

Author: brianjlai

airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py

@@ -806,9 +806,11 @@ def _init_mappings(self) -> None:
     def _create_stream_name_to_configured_stream(
         configured_catalog: Optional[ConfiguredAirbyteCatalog],
     ) -> Mapping[str, ConfiguredAirbyteStream]:
-        if configured_catalog is None:
-            return {}
-        return {stream.stream.name: stream for stream in configured_catalog.streams}
+        return (
+            {stream.stream.name: stream for stream in configured_catalog.streams}
+            if configured_catalog
+            else {}
+        )
 
     def create_component(
         self,

airbyte_cdk/sources/declarative/requesters/query_properties/query_properties.py

@@ -39,22 +39,21 @@ def get_request_property_chunks(
         :param configured_stream: The customer configured stream being synced which is needed to identify which
         record fields to query for and emit.
         """
+        configured_properties = self._get_configured_properties(configured_stream)
+
         fields: Union[Iterable[str], List[str]]
         if isinstance(self.property_list, PropertiesFromEndpoint):
             fields = self.property_list.get_properties_from_endpoint(stream_slice=stream_slice)
         else:
             fields = self.property_list if self.property_list else []
 
-        configured_properties = self._get_configured_properties(configured_stream)
-
         if self.property_chunking:
             yield from self.property_chunking.get_request_property_chunks(
                 property_fields=fields,
                 always_include_properties=self.always_include_properties,
                 configured_properties=configured_properties,
             )
         else:
-            # A schema might have no extra properties enabled which is valid and represented by an empty set
             if configured_properties is not None:
                 yield from [[field for field in fields if field in configured_properties]]
             else:
@@ -64,7 +63,13 @@ def get_request_property_chunks(
     def _get_configured_properties(
         configured_stream: Optional[ConfiguredAirbyteStream] = None,
     ) -> Optional[Set[str]]:
+        """
+        Returns the set of properties that have been selected for the configured stream. The intent being that
+        we should only query for selected properties not all since disabled properties are discarded.
+
+        When configured_stream is None, then there was no incoming catalog and all fields should be retrieved.
+        This is different from the empty set where the json_schema was empty and no schema fields were selected.
+        """
         if configured_stream:
-            # todo double check that configured catalog only contains enabled fields
             return set(configured_stream.stream.json_schema.get("properties", {}).keys())
         return None

unit_tests/connector_builder/test_property_chunking.py

@@ -186,7 +186,12 @@
                 "json_schema": {
                     "$schema": "http://json-schema.org/draft-07/schema#",
                     "type": "object",
-                    "properties": {},
+                    "properties": {
+                        "one": {"type": ["null", "string"]},
+                        "two": {"type": ["null", "string"]},
+                        "three": {"type": ["null", "string"]},
+                        "four": {"type": ["null", "string"]},
+                    },
                 },
                 "supported_sync_modes": ["full_refresh"],
                 "source_defined_cursor": False,

unit_tests/sources/declarative/requesters/query_properties/test_property_chunking.py

@@ -72,6 +72,7 @@ def test_get_request_property_chunks(
     property_limit,
     expected_property_chunks,
 ):
+    configured_properties = set(property_fields)
     property_fields = iter(property_fields)
     property_chunking = PropertyChunking(
         property_limit_type=property_limit_type,
@@ -83,7 +84,9 @@ def test_get_request_property_chunks(
 
     property_chunks = list(
         property_chunking.get_request_property_chunks(
-            property_fields=property_fields, always_include_properties=always_include_properties
+            property_fields=property_fields,
+            always_include_properties=always_include_properties,
+            configured_properties=configured_properties,
         )
     )
 
@@ -92,6 +95,54 @@ def test_get_request_property_chunks(
         assert property_chunks[i] == expected_property_chunk
 
 
+def test_get_request_property_chunks_empty_configured_properties():
+    expected_property_chunks = [["white", "lotus"]]
+
+    always_include_properties = ["white", "lotus"]
+    property_fields = ["maui", "taormina", "koh_samui", "saint_jean_cap_ferrat"]
+    configured_properties = set()
+    property_chunking = PropertyChunking(
+        property_limit_type=PropertyLimitType.property_count,
+        property_limit=3,
+        record_merge_strategy=GroupByKey(key="id", config=CONFIG, parameters={}),
+        config=CONFIG,
+        parameters={},
+    )
+    property_chunks = list(
+        property_chunking.get_request_property_chunks(
+            property_fields=property_fields,
+            always_include_properties=always_include_properties,
+            configured_properties=configured_properties,
+        )
+    )
+    assert property_chunks == expected_property_chunks
+
+
+def test_get_request_property_chunks_none_configured_properties():
+    expected_property_chunks = [
+        ["white", "lotus", "maui", "taormina"],
+        ["white", "lotus", "koh_samui", "saint_jean_cap_ferrat"],
+    ]
+
+    always_include_properties = ["white", "lotus"]
+    property_fields = ["maui", "taormina", "koh_samui", "saint_jean_cap_ferrat"]
+    property_chunking = PropertyChunking(
+        property_limit_type=PropertyLimitType.property_count,
+        property_limit=2,
+        record_merge_strategy=GroupByKey(key="id", config=CONFIG, parameters={}),
+        config=CONFIG,
+        parameters={},
+    )
+    property_chunks = list(
+        property_chunking.get_request_property_chunks(
+            property_fields=property_fields,
+            always_include_properties=always_include_properties,
+            configured_properties=None,
+        )
+    )
+    assert property_chunks == expected_property_chunks
+
+
 def test_get_merge_key():
     record = Record(stream_name="test", data={"id": "0"})
     property_chunking = PropertyChunking(