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

Discussion: API of ResourceIndex and ResourceIndex::update_all() Return Type #82

Open
tareknaser opened this issue Jul 21, 2024 · 1 comment

Comments

@tareknaser
Copy link
Collaborator

The return type of ResourceIndex::update_all() as currently implemented in #79 is:

/// Represents the result of an update operation on the ResourceIndex
#[derive(PartialEq, Debug)]
pub struct IndexUpdate<Id: ResourceId> {
    /// Resources that were added during the update
    added: HashMap<Id, IndexedResource<Id>>,
    /// Resources that were removed during the update
    removed: HashSet<Id>,
}

Here, added represents the new (either created or modified) resources in the index compared to the previous state, and removed represents the resources that were removed.

Problem Statement

We allow multiple resources to have the same ID to account for:

  • Resources with duplicate content
  • Resources with different content but the same ID (when using a non-cryptographic hash function)

Consider an index with 3 resources sharing the same ID (hash). If one of these resources is removed and update_all() is called, what should the return value be?

  • Should we consider the ID "removed" from the index? This wouldn't be accurate since the ID still exists in the index.
  • Should we not report that the ID was "removed"? This could mislead users into thinking there was no change in the current index state, which is incorrect.

Regardless of the decision, there should be a clear description of the decision taken and the expected API behavior for people using ResourceIndex as an external crate.

Let's discuss the best approach to handle this scenario and update the API documentation accordingly.

Related discussion: #79 (comment)

@kirillt
Copy link
Member

kirillt commented Jul 22, 2024

When new duplicate appeared, I would expect that we mention it in added field, but not in removed as we do with renamings. But if a duplicate is removed, we might need special field in the IndexUpdate structure.

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