holochain / scaffolding

Scaffolding tool to quickly generate and modify holochain applications
https://docs.rs/holochain_scaffolding_cli
221 stars 19 forks source link

Incorrect code generated for entry-type #335

Closed tibetsprague closed 3 months ago

tibetsprague commented 4 months ago

Using 0.3000.1 on macOS Sonoma

I generated a new Entry Type called contact and two parts of the generated code are causes errors:

For this code I get an error "cannot find value original_create_action in this scope"

                        EntryTypes::Contact(contact) => {
                            let original_app_entry = must_get_valid_record(
                                action.clone().original_action_address,
                            )?;
                            let original_contact = match Contact::try_from(
                                original_app_entry,
                            ) {
                                Ok(entry) => entry,
                                Err(e) => {
                                    return Ok(
                                        ValidateCallbackResult::Invalid(
                                            format!("Expected to get Contact from Record: {e:?}"),
                                        ),
                                    );
                                }
                            };
                            validate_update_contact(
                                action,
                                contact,
                                original_create_action,
                                original_contact,
                            )
                        }

For this code I get the error "cannot find value original_action in this scope"

                EntryTypes::Contact(original_contact) => {
                    validate_delete_contact(
                        delete_entry.clone().action,
                        original_action,
                        original_contact,
                    )
                }

And here is the full set of commands I did:

✔ Entry type name (snake_case): · contact
✔ Multiple integrity zomes were found in DNA relay, choose one: · relay_integrity

Which fields should the entry contain?

✔ Choose field type: · AgentPubKey
✔ Should a link from the AgentPubKey provided in this field also be created when entries of this type are created? · yes
✔ Which role does this agent play in the relationship ? (eg. "creator", "invitee") · contact
✔ Field name: · public_key

✔ Choose field type: · String
✔ Field name: · name
✔ Should this field be visible in the UI? · yes
✔ Choose widget to render this field: · TextField

✔ Choose field type: · String
✔ Field name: · avatar
✔ Should this field be visible in the UI? · yes
✔ Choose widget to render this field: · TextField

Chosen fields: public_key, name, avatar

✔ Which CRUD functions should be scaffolded (SPACE to select/unselect, ENTER to continue)? · Update, Delete
✔ Should a link from the original entry be created when this entry is updated? · Yes (more storage cost but better read performance, recommended)

Entry type contact scaffolded!
c12i commented 4 months ago

@tibetsprague could you share the full validate function?

tibetsprague commented 4 months ago
pub fn validate(op: Op) -> ExternResult<ValidateCallbackResult> {
    match op.flattened::<EntryTypes, LinkTypes>()? {
        FlatOp::StoreEntry(store_entry) => {
            match store_entry {
                OpEntry::CreateEntry { app_entry, action } => {
                    match app_entry {
                        EntryTypes::Config(config) => {
                            validate_create_config(
                                EntryCreationAction::Create(action),
                                config,
                            )
                        }
                        EntryTypes::Message(message) => {
                            validate_create_message(
                                EntryCreationAction::Create(action),
                                message,
                            )
                        }
                        EntryTypes::Contact(contact) => {
                            validate_create_contact(
                                EntryCreationAction::Create(action),
                                contact,
                            )
                        }
                    }
                }
                OpEntry::UpdateEntry { app_entry, action, .. } => {
                    match app_entry {
                        EntryTypes::Config(config) => {
                            validate_create_config(
                                EntryCreationAction::Update(action),
                                config,
                            )
                        }
                        EntryTypes::Message(message) => {
                            validate_create_message(
                                EntryCreationAction::Update(action),
                                message,
                            )
                        }
                        EntryTypes::Contact(contact) => {
                            validate_create_contact(
                                EntryCreationAction::Update(action),
                                contact,
                            )
                        }
                    }
                }
                _ => Ok(ValidateCallbackResult::Valid),
            }
        }
        FlatOp::RegisterUpdate(update_entry) => {
            match update_entry {
                OpUpdate::Entry { app_entry, action } => {
                    match app_entry {
                        // EntryTypes::Contact(contact) => {
                        //     let original_app_entry = must_get_valid_record(
                        //         action.clone().original_action_address,
                        //     )?;
                        //     let original_contact = match Contact::try_from(
                        //         original_app_entry,
                        //     ) {
                        //         Ok(entry) => entry,
                        //         Err(e) => {
                        //             return Ok(
                        //                 ValidateCallbackResult::Invalid(
                        //                     format!("Expected to get Contact from Record: {e:?}"),
                        //                 ),
                        //             );
                        //         }
                        //     };
                        //     validate_update_contact(
                        //         action,
                        //         contact,
                        //         original_create_action,
                        //         original_contact,
                        //     )
                        // }
                        EntryTypes::Message(message) => {
                            validate_update_message(action, message)
                        }
                        EntryTypes::Config(config) => {
                            validate_update_config(action, config)
                        }
                        _ => {
                            Ok(
                                ValidateCallbackResult::Invalid(
                                    "Original and updated entry types must be the same"
                                        .to_string(),
                                ),
                            )
                        }
                    }
                }
                _ => Ok(ValidateCallbackResult::Valid),
            }
        }
        FlatOp::RegisterDelete(delete_entry) => {
            match delete_entry {
                // EntryTypes::Contact(original_contact) => {
                //     validate_delete_contact(
                //         delete_entry.clone().action,
                //         original_action,
                //         original_contact,
                //     )
                // }
                OpDelete { action: _ } => Ok(ValidateCallbackResult::Valid),
                _ => Ok(ValidateCallbackResult::Valid),
            }
        }
        FlatOp::RegisterCreateLink {
            link_type,
            base_address,
            target_address,
            tag,
            action,
        } => {
            match link_type {
                LinkTypes::ConfigUpdates => {
                    validate_create_link_config_updates(
                        action,
                        base_address,
                        target_address,
                        tag,
                    )
                }
                LinkTypes::MessageUpdates => {
                    validate_create_link_message_updates(
                        action,
                        base_address,
                        target_address,
                        tag,
                    )
                }
                LinkTypes::AllMessages => {
                    validate_create_link_all_messages(
                        action,
                        base_address,
                        target_address,
                        tag,
                    )
                }
                LinkTypes::ContactToContacts => {
                    validate_create_link_contact_to_contacts(
                        action,
                        base_address,
                        target_address,
                        tag,
                    )
                }
                LinkTypes::ContactUpdates => {
                    validate_create_link_contact_updates(
                        action,
                        base_address,
                        target_address,
                        tag,
                    )
                }
            }
        }
        FlatOp::RegisterDeleteLink {
            link_type,
            base_address,
            target_address,
            tag,
            original_action,
            action,
        } => {
            match link_type {
                LinkTypes::ConfigUpdates => {
                    validate_delete_link_config_updates(
                        action,
                        original_action,
                        base_address,
                        target_address,
                        tag,
                    )
                }
                LinkTypes::MessageUpdates => {
                    validate_delete_link_message_updates(
                        action,
                        original_action,
                        base_address,
                        target_address,
                        tag,
                    )
                }
                LinkTypes::AllMessages => {
                    validate_delete_link_all_messages(
                        action,
                        original_action,
                        base_address,
                        target_address,
                        tag,
                    )
                }
                LinkTypes::ContactToContacts => {
                    validate_delete_link_contact_to_contacts(
                        action,
                        original_action,
                        base_address,
                        target_address,
                        tag,
                    )
                }
                LinkTypes::ContactUpdates => {
                    validate_delete_link_contact_updates(
                        action,
                        original_action,
                        base_address,
                        target_address,
                        tag,
                    )
                }
            }
        }
        FlatOp::StoreRecord(store_record) => {
            match store_record {
                OpRecord::CreateEntry { app_entry, action } => {
                    match app_entry {
                        EntryTypes::Config(config) => {
                            validate_create_config(
                                EntryCreationAction::Create(action),
                                config,
                            )
                        }
                        EntryTypes::Message(message) => {
                            validate_create_message(
                                EntryCreationAction::Create(action),
                                message,
                            )
                        }
                        EntryTypes::Contact(contact) => {
                            validate_create_contact(
                                EntryCreationAction::Create(action),
                                contact,
                            )
                        }
                    }
                }
                OpRecord::UpdateEntry {
                    original_action_hash,
                    app_entry,
                    action,
                    ..
                } => {
                    let original_record = must_get_valid_record(original_action_hash)?;
                    let original_action = original_record.action().clone();
                    let original_action = match original_action {
                        Action::Create(create) => EntryCreationAction::Create(create),
                        Action::Update(update) => EntryCreationAction::Update(update),
                        _ => {
                            return Ok(
                                ValidateCallbackResult::Invalid(
                                    "Original action for an update must be a Create or Update action"
                                        .to_string(),
                                ),
                            );
                        }
                    };
                    match app_entry {
                        EntryTypes::Config(config) => {
                            let result = validate_create_config(
                                EntryCreationAction::Update(action.clone()),
                                config.clone(),
                            )?;
                            if let ValidateCallbackResult::Valid = result {
                                let original_config: Option<Config> = original_record
                                    .entry()
                                    .to_app_option()
                                    .map_err(|e| wasm_error!(e))?;
                                let original_config = match original_config {
                                    Some(config) => config,
                                    None => {
                                        return Ok(
                                            ValidateCallbackResult::Invalid(
                                                "The updated entry type must be the same as the original entry type"
                                                    .to_string(),
                                            ),
                                        );
                                    }
                                };
                                validate_update_config(action, config)
                            } else {
                                Ok(result)
                            }
                        }
                        EntryTypes::Message(message) => {
                            let result = validate_create_message(
                                EntryCreationAction::Update(action.clone()),
                                message.clone(),
                            )?;
                            if let ValidateCallbackResult::Valid = result {
                                let original_message: Option<Message> = original_record
                                    .entry()
                                    .to_app_option()
                                    .map_err(|e| wasm_error!(e))?;
                                let original_message = match original_message {
                                    Some(message) => message,
                                    None => {
                                        return Ok(
                                            ValidateCallbackResult::Invalid(
                                                "The updated entry type must be the same as the original entry type"
                                                    .to_string(),
                                            ),
                                        );
                                    }
                                };
                                validate_update_message(action, message)
                            } else {
                                Ok(result)
                            }
                        }
                        EntryTypes::Contact(contact) => {
                            let result = validate_create_contact(
                                EntryCreationAction::Update(action.clone()),
                                contact.clone(),
                            )?;
                            if let ValidateCallbackResult::Valid = result {
                                let original_contact: Option<Contact> = original_record
                                    .entry()
                                    .to_app_option()
                                    .map_err(|e| wasm_error!(e))?;
                                let original_contact = match original_contact {
                                    Some(contact) => contact,
                                    None => {
                                        return Ok(
                                            ValidateCallbackResult::Invalid(
                                                "The updated entry type must be the same as the original entry type"
                                                    .to_string(),
                                            ),
                                        );
                                    }
                                };
                                validate_update_contact(
                                    action,
                                    contact,
                                    original_action,
                                    original_contact,
                                )
                            } else {
                                Ok(result)
                            }
                        }
                    }
                }
                OpRecord::DeleteEntry { original_action_hash, action, .. } => {
                    let original_record = must_get_valid_record(original_action_hash)?;
                    let original_action = original_record.action().clone();
                    let original_action = match original_action {
                        Action::Create(create) => EntryCreationAction::Create(create),
                        Action::Update(update) => EntryCreationAction::Update(update),
                        _ => {
                            return Ok(
                                ValidateCallbackResult::Invalid(
                                    "Original action for a delete must be a Create or Update action"
                                        .to_string(),
                                ),
                            );
                        }
                    };
                    let app_entry_type = match original_action.entry_type() {
                        EntryType::App(app_entry_type) => app_entry_type,
                        _ => {
                            return Ok(ValidateCallbackResult::Valid);
                        }
                    };
                    let entry = match original_record.entry().as_option() {
                        Some(entry) => entry,
                        None => {
                            if original_action.entry_type().visibility().is_public() {
                                return Ok(
                                    ValidateCallbackResult::Invalid(
                                        "Original record for a delete of a public entry must contain an entry"
                                            .to_string(),
                                    ),
                                );
                            } else {
                                return Ok(ValidateCallbackResult::Valid);
                            }
                        }
                    };
                    let original_app_entry = match EntryTypes::deserialize_from_type(
                        app_entry_type.zome_index,
                        app_entry_type.entry_index,
                        entry,
                    )? {
                        Some(app_entry) => app_entry,
                        None => {
                            return Ok(
                                ValidateCallbackResult::Invalid(
                                    "Original app entry must be one of the defined entry types for this zome"
                                        .to_string(),
                                ),
                            );
                        }
                    };
                    match original_app_entry {
                        EntryTypes::Config(original_config) => {
                            validate_delete_config(
                                action,
                                original_action,
                                original_config,
                            )
                        }
                        EntryTypes::Message(original_message) => {
                            validate_delete_message(
                                action,
                                original_action,
                                original_message,
                            )
                        }
                        EntryTypes::Contact(original_contact) => {
                            validate_delete_contact(
                                action,
                                original_action,
                                original_contact,
                            )
                        }
                    }
                }
                OpRecord::CreateLink {
                    base_address,
                    target_address,
                    tag,
                    link_type,
                    action,
                } => {
                    match link_type {
                        LinkTypes::ConfigUpdates => {
                            validate_create_link_config_updates(
                                action,
                                base_address,
                                target_address,
                                tag,
                            )
                        }
                        LinkTypes::MessageUpdates => {
                            validate_create_link_message_updates(
                                action,
                                base_address,
                                target_address,
                                tag,
                            )
                        }
                        LinkTypes::AllMessages => {
                            validate_create_link_all_messages(
                                action,
                                base_address,
                                target_address,
                                tag,
                            )
                        }
                        LinkTypes::ContactToContacts => {
                            validate_create_link_contact_to_contacts(
                                action,
                                base_address,
                                target_address,
                                tag,
                            )
                        }
                        LinkTypes::ContactUpdates => {
                            validate_create_link_contact_updates(
                                action,
                                base_address,
                                target_address,
                                tag,
                            )
                        }
                    }
                }
                OpRecord::DeleteLink { original_action_hash, base_address, action } => {
                    let record = must_get_valid_record(original_action_hash)?;
                    let create_link = match record.action() {
                        Action::CreateLink(create_link) => create_link.clone(),
                        _ => {
                            return Ok(
                                ValidateCallbackResult::Invalid(
                                    "The action that a DeleteLink deletes must be a CreateLink"
                                        .to_string(),
                                ),
                            );
                        }
                    };
                    let link_type = match LinkTypes::from_type(
                        create_link.zome_index,
                        create_link.link_type,
                    )? {
                        Some(lt) => lt,
                        None => {
                            return Ok(ValidateCallbackResult::Valid);
                        }
                    };
                    match link_type {
                        LinkTypes::ConfigUpdates => {
                            validate_delete_link_config_updates(
                                action,
                                create_link.clone(),
                                base_address,
                                create_link.target_address,
                                create_link.tag,
                            )
                        }
                        LinkTypes::MessageUpdates => {
                            validate_delete_link_message_updates(
                                action,
                                create_link.clone(),
                                base_address,
                                create_link.target_address,
                                create_link.tag,
                            )
                        }
                        LinkTypes::AllMessages => {
                            validate_delete_link_all_messages(
                                action,
                                create_link.clone(),
                                base_address,
                                create_link.target_address,
                                create_link.tag,
                            )
                        }
                        LinkTypes::ContactToContacts => {
                            validate_delete_link_contact_to_contacts(
                                action,
                                create_link.clone(),
                                base_address,
                                create_link.target_address,
                                create_link.tag,
                            )
                        }
                        LinkTypes::ContactUpdates => {
                            validate_delete_link_contact_updates(
                                action,
                                create_link.clone(),
                                base_address,
                                create_link.target_address,
                                create_link.tag,
                            )
                        }
                    }
                }
                OpRecord::CreatePrivateEntry { .. } => Ok(ValidateCallbackResult::Valid),
                OpRecord::UpdatePrivateEntry { .. } => Ok(ValidateCallbackResult::Valid),
                OpRecord::CreateCapClaim { .. } => Ok(ValidateCallbackResult::Valid),
                OpRecord::CreateCapGrant { .. } => Ok(ValidateCallbackResult::Valid),
                OpRecord::UpdateCapClaim { .. } => Ok(ValidateCallbackResult::Valid),
                OpRecord::UpdateCapGrant { .. } => Ok(ValidateCallbackResult::Valid),
                OpRecord::Dna { .. } => Ok(ValidateCallbackResult::Valid),
                OpRecord::OpenChain { .. } => Ok(ValidateCallbackResult::Valid),
                OpRecord::CloseChain { .. } => Ok(ValidateCallbackResult::Valid),
                OpRecord::InitZomesComplete { .. } => Ok(ValidateCallbackResult::Valid),
                _ => Ok(ValidateCallbackResult::Valid),
            }
        }
        FlatOp::RegisterAgentActivity(agent_activity) => {
            match agent_activity {
                OpActivity::CreateAgent { agent, action } => {
                    let previous_action = must_get_action(action.prev_action)?;
                    match previous_action.action() {
                        Action::AgentValidationPkg(
                            AgentValidationPkg { membrane_proof, .. },
                        ) => validate_agent_joining(agent, membrane_proof),
                        _ => {
                            Ok(
                                ValidateCallbackResult::Invalid(
                                    "The previous action for a `CreateAgent` action must be an `AgentValidationPkg`"
                                        .to_string(),
                                ),
                            )
                        }
                    }
                }
                _ => Ok(ValidateCallbackResult::Valid),
            }
        }
    }
}
tibetsprague commented 4 months ago

the broken parts are commented out

c12i commented 4 months ago

@tibetsprague Sorry for the delay.

Unfortunately, I am unable to reproduce this issue. Regarding the FlatOp::RegisterUpdate arm, it appears that the relevant code was either not generated or was manually deleted after being generated.

let original_action = must_get_action(action.clone().original_action_address)?
    .action()
    .to_owned();
let original_create_action = match EntryCreationAction::try_from(original_action) {
    Ok(action) => action,
    Err(e) => {
        return Ok(ValidateCallbackResult::Invalid(format!(
            "Expected to get EntryCreationAction from Action: {e:?}"
        )));
    }
};

match app_entry {
    EntryTypes::Contact(contact) => {
        let original_app_entry = must_get_valid_record(action.clone().original_action_address)?;
        let original_contact = match Contact::try_from(original_app_entry) {
            Ok(entry) => entry,
            Err(e) => {
                return Ok(ValidateCallbackResult::Invalid(format!(
                    "Expected to get Contact from Record: {e:?}"
                )));
            }
        };
        validate_update_contact(
            action,
            contact,
            original_create_action,
            original_contact,
        )
    }
}

In my tests, the above code is consistently generated, and I do not encounter any cannot find value original_create_action in this scope errors.

Similarly, the FlatOp::RegisterDelete arm is missing some code, which I can generate without issue:

let original_action_hash = delete_entry.clone().action.deletes_address;
let original_record = must_get_valid_record(original_action_hash)?;
let original_record_action = original_record.action().clone();
let original_action = match EntryCreationAction::try_from(original_record_action) {
    Ok(action) => action,
    Err(e) => {
        return Ok(ValidateCallbackResult::Invalid(format!(
            "Expected to get EntryCreationAction from Action: {e:?}"
        )));
    }
};
let app_entry_type = match original_action.entry_type() {
    EntryType::App(app_entry_type) => app_entry_type,
    _ => {
        return Ok(ValidateCallbackResult::Valid);
    }
};
let entry = match original_record.entry().as_option() {
    Some(entry) => entry,
    None => {
        return Ok(ValidateCallbackResult::Invalid(
            "Original record for a delete must contain an entry".to_string(),
        ));
    }
};
let original_app_entry = match EntryTypes::deserialize_from_type(
    app_entry_type.zome_index,
    app_entry_type.entry_index,
    entry,
)? {
    Some(app_entry) => app_entry,
    None => {
        return Ok(ValidateCallbackResult::Invalid(
            "Original app entry must be one of the defined entry types for this zome".to_string(),
        ));
    }
};

match original_app_entry {
    EntryTypes::Contact(original_contact) => validate_delete_contact(
        delete_entry.clone().action,
        original_action,
        original_contact,
    ),
}

The FlatOp::RegisterDelete arm in particular contains a match delete_entry arm that is no longer generated with version holochain_scaffolding_cli 0.3000.1.

Could you provide some additional context to help clarify the issue?

  1. Was the original code scaffolded with holochain_scaffolding_cli 0.2001.x?
  2. Was there any manual modification to the validate function prior to generating the entry type?
tibetsprague commented 4 months ago

So I tried running this with 0.3000.1 but because of this issue https://github.com/holochain/scaffolding/issues/328 I wasn't able to do that by just running hc-scaffold, instead I looked through the /nix/store for the correct version and directly ran it like this /nix/store/dga5w7vzk1rswbs021da47fkyzw3lqya-hc-scaffold-0.3000.1/bin/hc-scaffold Do you think there could be an issue running it this way, where maybe its calling the wrong version of some dependency or library?

Unfortunately the other solution listed in that ticket above where I tweak the flake.nix to get the correct version of hc-scaffold isn't working for me now because of this bug https://github.com/holochain/holochain/issues/4113 🤦‍♂️

tibetsprague commented 4 months ago

I dont know exactly which version the original code was scaffolded with. It was scaffolded in April with github:holochain/holochain/?dir=versions/weekly

And yes there have been some modifications to validate since then

Let me know if you have other ideas for now about how to get this to work.

c12i commented 3 months ago

Do you think there could be an issue running it this way, where maybe its calling the wrong version of some dependency or library?

I think running the binary directly from the nix store is generally fine, but I am not entirely sure of the repercussions.

Nevertheless, I think this is an issue that should be addressed in the p2p-shipyard flake. Opened this issue on the p2p shipyard repo to prevent locking the scaffolding version at this level.

c12i commented 3 months ago

It was scaffolded in April with github:holochain/holochain/?dir=versions/weekly.

I see. Around that time, there was a breaking change in the validation logic introduced by Holochain (https://github.com/holochain/holochain/pull/3627), which specifically affected the problematic code. If the original code was scaffolded with a version prior to this breaking change, it is likely that the validation code was manually modified to accommodate the change.

As a workaround, the most reasonable solution would be to manually define the undeclared variables.

For more details, please refer to the discussion here: https://github.com/holochain/scaffolding/issues/335#issuecomment-2242539401.

tibetsprague commented 3 months ago

ok, I manually added the code and it worked. thanks!