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

Upsert updates wrong objects in the collection #172

Open
pawelbaran opened this issue Feb 2, 2023 · 3 comments
Open

Upsert updates wrong objects in the collection #172

pawelbaran opened this issue Feb 2, 2023 · 3 comments
Assignees
Labels
type:bug Error or unexpected behaviour

Comments

@pawelbaran
Copy link
Member

pawelbaran commented Feb 2, 2023

Description:

Using Mongo_Adapter in upsert mode (PushType.UpdateOrCreateOnly) yields seriously wrong results: due to the fact that the pushed objects are matched by __Tag__ instead of BHoM_Guid, any sort of wonderful updates happens - see the test files.

Test files:

On SharePoint.

@pawelbaran pawelbaran added the type:bug Error or unexpected behaviour label Feb 2, 2023
pawelbaran added a commit that referenced this issue Feb 2, 2023
@adecler
Copy link
Member

adecler commented Feb 3, 2023

They shouldn't be matched by BHoM_Guid because those are changing every time the component creating the objects is re-rerun.

@pawelbaran
Copy link
Member Author

pawelbaran commented May 24, 2023

To summarise the discussion under #173, Mongo_Adapter in its current state bears a range of issues that make it virtually unusable for the general case:

  • need to manually set individual tag to each object being pushed (different from any contemporary ORMs, also different from other BHoM adapters)
  • silent failures
  • inability to meaningfully push any objects that do not inherit from BHoMObject because of lack of Tags property (they cannot be tracked without building custom expressions)
  • data gets corrupt if someone without the tribal knowledge sets duplicate tags (e.g. empty tag) and attempts to do any sort of update

Leaving this comment as a reference for the time when there is interest in improving the adapter.

@pawelbaran pawelbaran removed their assignment May 24, 2023
@adecler
Copy link
Member

adecler commented Jan 11, 2024

From discussion with @pawelbaran :

  • The issue right now is the tag parameter is only defined as an input of the Push. We could add a parameter to the PushConfig to define what property of the objects is used to define the tag. If not defined in the PushConfig, then the input tag is used as before.
  • We also touched on the possibility of having multiple tags on objects so that they can be retrieved and updated from different view points (i.e. each object belongs to multiple categories). Since BHoM objects already have a Tag property, this could be a likely candidate for this feature. Again, we would need to specify in the PushConfig that the Tag property needs to be used to remain compatible with exiting behaviour and keep supporting the simple user case. We also need to look into performance impact since multiple tags means search in an array instead of a simple string matching.
  • Finally we also need to look at supporting indexes for performance reasons. For that, we would need to have another property in the PushConfig that defines what property is used as key (if any). We would also need an additional PullRequest type that allows to pull by index.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Error or unexpected behaviour
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants