Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AMI update: merge sbf json rather than replace #41

Closed
patdunlavey opened this issue Nov 10, 2021 · 4 comments
Closed

AMI update: merge sbf json rather than replace #41

patdunlavey opened this issue Nov 10, 2021 · 4 comments

Comments

@patdunlavey
Copy link
Contributor

patdunlavey commented Nov 10, 2021

There is a lovely thing we see when editing an object via webform, that existing json data that is not overwritten by the webform is preserved. The previous json and the new json are recursively merged.

It would be nice to have this behavior (optional, or always) when processing an update AMI set.

Without this feature, an update AMI set must provide all data, rather than just the columns that are required (node_uuid, etc) and the columns being edited. Haven't checked, but it might even be necessary currently to provide the automatically generated as: elements.

@DiegoPino
Copy link
Member

DiegoPino commented Nov 10, 2021

@patdunlavey and @dmer yep! Why don't you give a pull a try? The code is really there even the options (commented out on the form) are readu

https://github.com/esmero/ami/blob/ISSUE-14/src/Plugin/QueueWorker/IngestADOQueueWorker.php#L448-L458

just needs some testing and a few ifs.

First, as: and ap: keys from original (files) and entity mappings are already preserved... so that is kinda done (have you tested it? Maybe I missed something?)

$original_file_mappings = $original_value['ap:entitymapping']['entity:file'] ?? [];
$original_node_mappings = $original_value['ap:entitymapping']['entity:node'] ?? [];
foreach ($original_file_mappings as $filekey) {
if (!in_array($filekey, $processed_metadata['ap:entitymapping']['entity:file'])) {
$processed_metadata[$filekey] = $original_value[$filekey] ?? [];
$processed_metadata['ap:entitymapping']['entity:file'][] = $filekey;
}
}
foreach ($original_node_mappings as $nodekey) {
if (!in_array($nodekey, $processed_metadata['ap:entitymapping']['entity:node'])) {
$processed_metadata[$nodekey] = $original_value[$nodekey] ?? [];
$processed_metadata['ap:entitymapping']['entity:node'][] = $nodekey;
}
}
// Really not needed?
$processed_metadata['ap:entitymapping']['entity:node'] = array_unique($processed_metadata['ap:entitymapping']['entity:node']);
$processed_metadata['ap:entitymapping']['entity:file'] = array_unique($processed_metadata['ap:entitymapping']['entity:file']);
// Copy directly all as:mediatype into the child, the File Persistent Event will clean this up if redundant.
foreach(StrawberryfieldJsonHelper::AS_FILE_TYPE as $as_file_type) {
if (isset($original_value[$as_file_type])) {
$processed_metadata[$as_file_type] = $original_value[$as_file_type];
}
}

And then, the actual patching of JSON/Array:

This part here (queue worker) is the done that deals with "not create" (update and/or patch)

else {
$vid = $this->entityTypeManager
->getStorage('node')
->getLatestRevisionId($existing_object->id());
$node = $vid ? $this->entityTypeManager->getStorage('node')
->loadRevision($vid) : $existing[0];
/** @var \Drupal\Core\Field\FieldItemInterface $field*/
$field = $node->get($field_name);
if ($status && is_string($status)) {
$node->set('moderation_state', $status);
$status = 0;
}
/** @var \Drupal\strawberryfield\Field\StrawberryFieldItemList $field */
if (!$field->isEmpty()) {
/** @var $field \Drupal\Core\Field\FieldItemList */
foreach ($field->getIterator() as $delta => $itemfield) {
/** @var \Drupal\strawberryfield\Plugin\Field\FieldType\StrawberryFieldItem $itemfield */
if ($field_name_offset == $delta) {
$original_value = $itemfield->provideDecoded(TRUE);
// Now calculate what we need to do here regarding files/node mappings
$original_file_mappings = $original_value['ap:entitymapping']['entity:file'] ?? [];
$original_node_mappings = $original_value['ap:entitymapping']['entity:node'] ?? [];
foreach ($original_file_mappings as $filekey) {
if (!in_array($filekey, $processed_metadata['ap:entitymapping']['entity:file'])) {
$processed_metadata[$filekey] = $original_value[$filekey] ?? [];
$processed_metadata['ap:entitymapping']['entity:file'][] = $filekey;
}
}
foreach ($original_node_mappings as $nodekey) {
if (!in_array($nodekey, $processed_metadata['ap:entitymapping']['entity:node'])) {
$processed_metadata[$nodekey] = $original_value[$nodekey] ?? [];
$processed_metadata['ap:entitymapping']['entity:node'][] = $nodekey;
}
}
// Really not needed?
$processed_metadata['ap:entitymapping']['entity:node'] = array_unique($processed_metadata['ap:entitymapping']['entity:node']);
$processed_metadata['ap:entitymapping']['entity:file'] = array_unique($processed_metadata['ap:entitymapping']['entity:file']);
// Copy directly all as:mediatype into the child, the File Persistent Event will clean this up if redundant.
foreach(StrawberryfieldJsonHelper::AS_FILE_TYPE as $as_file_type) {
if (isset($original_value[$as_file_type])) {
$processed_metadata[$as_file_type] = $original_value[$as_file_type];
}
}
$this->patchJson($original_value, $processed_metadata);
$itemfield->setMainValueFromArray($processed_metadata);
break;

And because of that I left there hanging for someone to help me out (limited hands) a line of code

$this->patchJson($original_value, $processed_metadata);

if can be wrapper with an if ($op=='patch') { }

So what is that $this->patchJson should do?

it can generate a diff (original json v/s new json and through that a set of JSON Patch instructions). Then you can either apply the patch over the original OR remove all JSON Patch instructions that "remove" values, etc. Or you can array crazy and do the logic by merging the arrays recursively. Issue with that is that if the Key (specially the as:ones) are in both, it will actually add extra values, not replace the old values with the new ones...so you may avoid as: and ap: keys completely (by unsetting them before the operation)

Since right now patchJson is not returning nor doing anything you might want to debug first there and then decide based on "if my patched empty? or things like that" if you use that to set the entity field.

Here are the docs:

https://github.com/swaggest/json-diff

let me know because I'm merging AMI tomorrow morning (release is done)

@patdunlavey
Copy link
Contributor Author

Thanks very much @DiegoPino ! Do not hold up merge/release. I won't get to this quickly enough, but I will take a whack at it in the next day or two.

@DiegoPino
Copy link
Member

Great! Let me know how that goes. Happy to help. Its really (my guess) no more than 10-20 lines of code. I promise!

@DiegoPino
Copy link
Member

Closing. I don't think a pull request came in. So might not be a priority for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants