Merge branch 'master' into fix-o2m-id-field-handling-rebased

Author: bosd

src/odoo_data_flow/import_threaded.py

@@ -579,8 +579,19 @@ def _handle_create_error(
     error_str = str(create_error)
     error_str_lower = error_str.lower()
 
-    # Handle database connection pool exhaustion errors
+    # Handle constraint violation errors (e.g., XML ID space constraint)
     if (
+        "constraint" in error_str_lower
+        or "check constraint" in error_str_lower
+        or "nospaces" in error_str_lower
+        or "violation" in error_str_lower
+    ):
+        error_message = f"Constraint violation in row {i + 1}: {create_error}"
+        if "Fell back to create" in error_summary:
+            error_summary = "Database constraint violation detected"
+
+    # Handle database connection pool exhaustion errors
+    elif (
         "connection pool is full" in error_str_lower
         or "too many connections" in error_str_lower
         or "poolerror" in error_str_lower
@@ -670,13 +681,17 @@ def _create_batch_individually(  # noqa: C901
                 )
 
             source_id = line[uid_index]
+            # Sanitize source_id to ensure it's a valid XML ID
+            from .lib.internal.tools import to_xmlid
+            sanitized_source_id = to_xmlid(source_id)
+            
             # 1. SEARCH BEFORE CREATE
             existing_record = model.browse().env.ref(
-                f"__export__.{source_id}", raise_if_not_found=False
+                f"__export__.{sanitized_source_id}", raise_if_not_found=False
             )
 
             if existing_record:
-                id_map[source_id] = existing_record.id
+                id_map[sanitized_source_id] = existing_record.id
                 continue
 
             # 2. PREPARE FOR CREATE
@@ -713,7 +728,7 @@ def _create_batch_individually(  # noqa: C901
             log.debug(f"Converted vals keys: {list(converted_vals.keys())}")
 
             new_record = model.create(converted_vals, context=context)
-            id_map[source_id] = new_record.id
+            id_map[sanitized_source_id] = new_record.id
         except IndexError as e:
             error_str_lower = str(e).lower()
 
@@ -916,17 +931,120 @@ def _execute_load_batch(  # noqa: C901
             log.debug(f"Attempting `load` for chunk of batch {batch_number}...")
 
             res = model.load(load_header, load_lines, context=context)
+
             if res.get("messages"):
                 error = res["messages"][0].get("message", "Batch load failed.")
-                raise ValueError(error)
-
+                # Don't raise immediately, log and continue to capture in fail file
+                log.error(f"Odoo server error during load: {error}")
+                
             created_ids = res.get("ids", [])
-            if len(created_ids) != len(load_lines):
-                raise ValueError("Record count mismatch after load.")
+            log.debug(f"Expected records: {len(sanitized_load_lines)}, Created records: {len(created_ids)}")
+            
+            # Always log detailed information about record creation
+            if len(created_ids) != len(sanitized_load_lines):
+                log.warning(
+                    f"Record creation mismatch: Expected {len(sanitized_load_lines)} records, "
+                    f"but only {len(created_ids)} were created"
+                )
+                if len(created_ids) == 0:
+                    log.error(
+                        f"No records were created in this batch of {len(sanitized_load_lines)}. "
+                        f"This may indicate silent failures in the Odoo load operation. "
+                        f"Check Odoo server logs for validation errors."
+                    )
+                    # Log the actual data being sent for debugging
+                    if sanitized_load_lines:
+                        log.debug(f"First few lines being sent:")
+                        for i, line in enumerate(sanitized_load_lines[:3]):
+                            log.debug(f"  Line {i}: {dict(zip(load_header, line))}")
+                else:
+                    log.warning(
+                        f"Partial record creation: {len(created_ids)}/{len(sanitized_load_lines)} "
+                        f"records were created. Some records may have failed validation."
+                    )
+            # Check for any Odoo server errors in the response that should halt processing
+            if res.get("messages"):
+                for message in res["messages"]:
+                    msg_type = message.get("type", "unknown")
+                    msg_text = message.get("message", "")
+                    if msg_type == "error":
+                        # Only raise for actual errors, not warnings
+                        log.error(f"Load operation returned fatal error: {msg_text}")
+                        raise ValueError(msg_text)
+                    elif msg_type in ["warning", "info"]:
+                        log.warning(f"Load operation returned {msg_type}: {msg_text}")
+                    else:
+                        log.info(f"Load operation returned {msg_type}: {msg_text}")
 
+            created_ids = res.get("ids", [])
+            log.debug(f"Expected records: {len(sanitized_load_lines)}, Created records: {len(created_ids)}")
+            
+            # Always log detailed information about record creation
+            if len(created_ids) != len(sanitized_load_lines):
+                log.warning(
+                    f"Record creation mismatch: Expected {len(sanitized_load_lines)} records, "
+                    f"but only {len(created_ids)} were created"
+                )
+                if len(created_ids) == 0:
+                    log.error(
+                        f"No records were created in this batch of {len(sanitized_load_lines)}. "
+                        f"This may indicate silent failures in the Odoo load operation. "
+                        f"Check Odoo server logs for validation errors."
+                    )
+                    # Log the actual data being sent for debugging
+                    if sanitized_load_lines:
+                        log.debug(f"First few lines being sent:")
+                        for i, line in enumerate(sanitized_load_lines[:3]):
+                            log.debug(f"  Line {i}: {dict(zip(load_header, line))}")
+                else:
+                    log.warning(
+                        f"Partial record creation: {len(created_ids)}/{len(sanitized_load_lines)} "
+                        f"records were created. Some records may have failed validation."
+                    )
+            
+            # Instead of raising an exception, capture failures for the fail file
+            # But still create what records we can
+            if res.get("messages"):
+                # Extract error information and add to failed_lines to be written to fail file
+                error_msg = res["messages"][0].get("message", "Batch load failed.")
+                log.error(f"Capturing load failure for fail file: {error_msg}")
+                # We'll add the failed lines to aggregated_failed_lines at the end
+            
+            # Use sanitized IDs for the id_map to match what was actually sent to Odoo
             id_map = {
-                line[uid_index]: created_ids[i] for i, line in enumerate(current_chunk)
+                to_xmlid(line[uid_index]): created_ids[i] if i < len(created_ids) else None 
+                for i, line in enumerate(current_chunk)
             }
+            
+            # Remove None entries (failed creations) from id_map
+            id_map = {k: v for k, v in id_map.items() if v is not None}
+            
+            # Log id_map information for debugging
+            log.debug(f"Created {len(id_map)} records in batch {batch_number}")
+            if id_map:
+                log.debug(f"Sample id_map entries: {dict(list(id_map.items())[:3])}")
+            else:
+                log.warning(f"No id_map entries created for batch {batch_number}")
+            
+            # Capture failed lines for writing to fail file
+            successful_count = len(created_ids)
+            total_count = len(sanitized_load_lines)
+            
+            if successful_count < total_count:
+                failed_count = total_count - successful_count
+                log.info(f"Capturing {failed_count} failed records for fail file")
+                # Add error information to the lines that failed
+                for i, line in enumerate(current_chunk):
+                    # Check if this line corresponds to a created record
+                    if i >= len(created_ids) or created_ids[i] is None:
+                        # This record failed, add it to failed_lines with error info
+                        error_msg = "Record creation failed"
+                        if res.get("messages"):
+                            error_msg = res["messages"][0].get("message", error_msg)
+                        
+                        failed_line = list(line) + [f"Load failed: {error_msg}"]
+                        aggregated_failed_lines.append(failed_line)
+            
             aggregated_id_map.update(id_map)
             lines_to_process = lines_to_process[chunk_size:]
 

src/odoo_data_flow/lib/relational_import.py

@@ -188,6 +188,15 @@ def _query_relation_info_from_odoo(
     Returns:
         A tuple of (relation_table, relation_field) or None if not found
     """
+    # Early return for self-referencing fields to avoid constraint errors
+    # These should be handled by the hardcoded mappings in _derive_relation_info
+    if model == related_model_fk:
+        log.debug(
+            f"Skipping ir.model.relation query for self-referencing field "
+            f"between '{model}' and '{related_model_fk}'"
+        )
+        return None
+
     try:
         # Get connection to Odoo
         if isinstance(config, dict):
@@ -201,6 +210,8 @@ def _query_relation_info_from_odoo(
 
         # Search for relations involving both models
         # We need to check both orders since the relation could be defined either way
+        # Note: The field names in ir.model.relation may vary by Odoo version
+        # Common field names are: model, comodel_id, or model_id for the related fields
         domain = [
             "|",
             "&",
@@ -235,6 +246,18 @@ def _query_relation_info_from_odoo(
             )
             return None
 
+    except ValueError as ve:
+        # Handle specific field validation errors in Odoo expressions
+        if "Invalid field" in str(ve) and "ir.model.relation" in str(ve):
+            log.warning(
+                f"Field validation error querying ir.model.relation: {ve}. "
+                f"This may be due to incorrect field names in the domain query."
+            )
+            # Fall back to derivation logic when we can't query the relation table
+            return None
+        else:
+            # Re-raise other ValueErrors
+            raise
     except Exception as e:
         log.warning(
             f"Failed to query ir.model.relation for models '{model}' and "
@@ -256,6 +279,17 @@ def _derive_relation_info(
     Returns:
         A tuple of (relation_table, relation_field)
     """
+    # Hardcoded mappings for known self-referencing fields
+    known_self_referencing_fields = {
+        ('product.template', 'optional_product_ids'): ('product_optional_rel', 'product_template_id'),
+        # Add more known self-referencing fields here as needed
+    }
+    
+    # Check if we have a known mapping for this field
+    key = (model, field)
+    if key in known_self_referencing_fields:
+        return known_self_referencing_fields[key]
+    
     # Derive relation table name (typically follows pattern: model1_model2_rel)
     # with models sorted alphabetically for canonical naming
     models = sorted([model.replace(".", "_"), related_model_fk.replace(".", "_")])
@@ -858,12 +892,22 @@ def run_write_o2m_tuple_import(
     failed_records_to_report = []
 
     # Filter for rows that actually have data in the o2m field
-    # Check if the field with /id suffix exists (common for relation fields)
-    actual_field_name = field
-    if f"{field}/id" in source_df.columns:
-        actual_field_name = f"{field}/id"
-
-    o2m_df = source_df.filter(pl.col(actual_field_name).is_not_null())
+    # Handle both direct field names and /id suffixed field names
+    actual_field = field
+    if field not in source_df.columns:
+        # Check if the field with /id suffix exists (common for relation fields)
+        field_with_id = f"{field}/id"
+        if field_with_id in source_df.columns:
+            log.debug(f"Using field '{field_with_id}' instead of '{field}' for O2M filtering")
+            actual_field = field_with_id
+        else:
+            log.error(
+                f"Field '{field}' not found in source DataFrame. "
+                f"Available columns: {list(source_df.columns)}"
+            )
+            return False
+    
+    o2m_df = source_df.filter(pl.col(actual_field).is_not_null())
 
     for record in o2m_df.iter_rows(named=True):
         parent_external_id = record["id"]