mattp5657 opened a new issue, #4307:
URL: https://github.com/apache/iggy/issues/4307
### Description
Eight `apply_field_transformations`/`apply_field_mappings` functions
(encoders, decoders, transforms) rename JSON keys the same way:
iterate the map by reference, clone every key/value into a side
buffer, `clear()` the map, reinsert from the buffer.
```rust
fn apply_field_transformations(&self, payload: Payload) -> Result<Payload,
Error> {
if let Some(mappings) = &self.config.field_mappings {
match payload {
Payload::Json(json_value) => {
if let simd_json::OwnedValue::Object(mut map) = json_value {
let mut new_entries = Vec::new();
for (key, value) in map.iter() {
if let Some(new_key) = mappings.get(key) {
new_entries.push((new_key.clone(),
value.clone()));
} else {
new_entries.push((key.clone(), value.clone()));
}
}
map.clear();
for (key, value) in new_entries {
map.insert(key, value);
}
Ok(Payload::Json(simd_json::OwnedValue::Object(map)))
} else {
Ok(Payload::Json(json_value))
}
}
other => Ok(other),
}
} else {
Ok(payload)
}
}
```
`map` is owned locally and emptied two lines after the clone. There's
no new type or shape to build here: it's the same `halfbrown::HashMap`
before and after, so cloning every key and value into a side buffer
just to `.clear()` and refill that same map a few lines later copies
data that was already exclusively owned and about to be dropped
anyway. `.drain()` moves entries out directly instead, which is all
`.clear()` was going to do to them regardless. Opt-in cost: only runs
when `field_mappings` is configured.
### Affected area / component
Connectors
### Proposed solution
```rust
if let simd_json::OwnedValue::Object(mut map) = json_value {
let new_entries: Vec<(String, simd_json::OwnedValue)> = map
.drain()
.map(|(key, value)| match mappings.get(&key) {
Some(new_key) => (new_key.clone(), value),
None => (key, value),
})
.collect();
for (key, value) in new_entries {
map.insert(key, value);
}
Ok(Payload::Json(simd_json::OwnedValue::Object(map)))
} else {
Ok(Payload::Json(json_value))
}
```
No `Extend` on `halfbrown::HashMap`; reinsert via the `for` loop.
Apply to all 8 sites (adjust lookup line for the two noted above); no
signature changes. Cover rename/no-rename branches in tests.
Also 6 of the 8 sites are already byte-identical, so factoring them
into one shared helper would be a reasonable cleanup once this fix lands.
### Alternatives considered
_No response_
### Contribution
- [x] I'm willing to submit a pull request to implement this feature
### Good first issue
- [ ] I think this could be a good first issue for a new contributor
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
