[CORRUPTED] Synthetic Benchmark PR #138650 - Make AggregateMetricDoubleFieldType immutable

[tomerqodo]

User description

Benchmark PR elastic#138650

Type: Corrupted (contains bugs)

Original PR Title: Make AggregateMetricDoubleFieldType immutable
Original PR Description: unknown
Original PR URL: elastic#138650

---

PR Type

Enhancement

---

Description

• Make AggregateMetricDoubleFieldType immutable by converting mutable fields to final

• Move field initialization from setters to constructor parameters

• Remove setter methods (setMetricFields, setDefaultMetric, addMetricField)

• Update all test files to use new immutable constructor pattern

• Remove unused IndexType import from mapper

---

Diagram Walkthrough

flowchart LR
  A["Mutable AggregateMetricDoubleFieldType<br/>with setters"] -->|"Convert to immutable"| B["Immutable AggregateMetricDoubleFieldType<br/>with final fields"]
  B -->|"Update constructor"| C["Constructor accepts all<br/>required parameters"]
  D["Test files"] -->|"Refactor to use<br/>new constructor"| C

Loading

File Walkthrough

	Relevant files
Enhancement	AggregateMetricDoubleFieldMapper.java Make AggregateMetricDoubleFieldType immutable                        x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapper.java Convert metricFields and defaultMetric fields from mutable to final Refactor constructor to accept all initialization parameters including metricType, defaultMetric, metricFields, and meta Remove setter methods: setMetricFields(), setDefaultMetric(), and addMetricField() Update builder to pass all parameters to constructor instead of using setters Remove unused IndexType import +16/-34
Tests	AggregateMetricBackedAvgAggregatorTests.java Update test to use immutable constructor                                  x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedAvgAggregatorTests.java Add imports for EnumMap and Map Refactor createDefaultFieldType() to build metricFields map before constructor call Replace setter calls with direct constructor invocation passing all parameters +5/-5      AggregateMetricBackedMaxAggregatorTests.java Update test to use immutable constructor                                  x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMaxAggregatorTests.java Add imports for EnumMap and Map Refactor createDefaultFieldType() to build metricFields map before constructor call Replace setter calls with direct constructor invocation passing all parameters +5/-5      AggregateMetricBackedMinAggregatorTests.java Update test to use immutable constructor                                  x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMinAggregatorTests.java Add imports for EnumMap and Map Refactor createDefaultFieldType() to build metricFields map before constructor call Replace setter calls with direct constructor invocation passing all parameters +5/-5      AggregateMetricBackedSumAggregatorTests.java Update test to use immutable constructor                                  x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedSumAggregatorTests.java Add imports for EnumMap and Map Refactor createDefaultFieldType() to build metricFields map before constructor call Replace setter calls with direct constructor invocation passing all parameters +5/-5      AggregateMetricBackedValueCountAggregatorTests.java Update test to use immutable constructor                                  x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedValueCountAggregatorTests.java Add imports for EnumMap and Map Refactor createDefaultFieldType() to build metricFields map before constructor call Replace setter calls with direct constructor invocation passing all parameters +5/-5      AggregateMetricDoubleFieldTypeTests.java Update test to use immutable constructor                                  x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldTypeTests.java Add import for EnumMap Refactor createDefaultFieldType() to build metricFields map before constructor call Replace setter calls with direct constructor invocation passing all parameters +6/-9

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No audit logs: The new immutable constructor and builder wiring add no audit/event logging for critical actions, but given this is mapper/type construction code and tests, it may not require audit logs; needs human verification. Referred Code AggregateMetricDoubleFieldType metricFieldType = new AggregateMetricDoubleFieldType( context.buildFullName(leafName()), timeSeriesMetric.getValue(), metrics.getValue().iterator().next(), metricFields, meta.getValue() ); return new AggregateMetricDoubleFieldMapper(leafName(), metricFieldType, metricMappers, builderParams(this, context), this); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Null/edge checks: The new constructor uses metricFields.values().iterator().next() and defaultMetric without visible null/empty validation, which could cause runtime errors if inputs are invalid; may be validated elsewhere. Referred Code public AggregateMetricDoubleFieldType( String name, MetricType metricType, Metric defaultMetric, EnumMap<Metric, NumberFieldMapper.NumberFieldType> metricFields, Map<String, String> meta ) { super(name, metricFields.values().iterator().next().indexType(), false, meta); this.metricType = metricType; this.defaultMetric = defaultMetric; this.metricFields = metricFields; } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Input validation: The refactor introduces constructor parameters (metricType, defaultMetric, metricFields, meta) without visible validation or defensive copying, which could allow invalid or mutable inputs; may be enforced by the builder or framework. Referred Code public AggregateMetricDoubleFieldType( String name, MetricType metricType, Metric defaultMetric, EnumMap<Metric, NumberFieldMapper.NumberFieldType> metricFields, Map<String, String> meta ) { super(name, metricFields.values().iterator().next().indexType(), false, meta); this.metricType = metricType; this.defaultMetric = defaultMetric; this.metricFields = metricFields; } Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Use correct default metric value In the AggregateMetricDoubleFieldType constructor call, replace metrics.getValue().iterator().next() with defaultMetric.getValue() to pass the correct default metric. This ensures the user-configured or logically derived default metric is used. x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapper.java [253-259] AggregateMetricDoubleFieldType metricFieldType = new AggregateMetricDoubleFieldType( context.buildFullName(leafName()), timeSeriesMetric.getValue(), - metrics.getValue().iterator().next(), + defaultMetric.getValue(), metricFields, meta.getValue() ); Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a bug where the wrong default metric is used, which would ignore the user-configured default_metric and instead use the first metric in the enum's natural order.	High
More