chore: rename mongodb client functions from [CRUD]Entity to [CRUD]Metadata#480
Conversation
There was a problem hiding this comment.
Code Review
This pull request renames the CRUD operations in the MongoDB repository from 'Entity' to 'Metadata' (e.g., CreateMetadata, ReadMetadata) to better reflect their specific purpose. The changes span the repository implementation, the metadata handler, and associated tests. Feedback was provided to update the accompanying function comments to ensure they consistently refer to 'metadata' instead of 'entity' or 'attributes,' maintaining clarity throughout the codebase.
There was a problem hiding this comment.
Pull request overview
This PR renames the MongoDB repository CRUD-style methods from *Entity to *Metadata to better reflect that the MongoDB collection is being used for metadata persistence rather than full entity lifecycle management (per Issue #340).
Changes:
- Renamed
CreateEntity/ReadEntity/UpdateEntity/DeleteEntitytoCreateMetadata/ReadMetadata/UpdateMetadata/DeleteMetadatain the MongoDB repository. - Updated the Mongo metadata handler to call the renamed repository methods.
- Updated the MongoDB repository tests to use the renamed methods.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| opengin/core-api/db/repository/mongo/mongodb_client.go | Renames MongoRepository CRUD methods to *Metadata variants. |
| opengin/core-api/db/repository/mongo/mongodb_client_test.go | Updates tests to call the renamed *Metadata methods. |
| opengin/core-api/db/repository/mongo/metadata_handler.go | Updates metadata handler to use ReadMetadata/CreateMetadata/UpdateMetadata. |
Comments suppressed due to low confidence (1)
opengin/core-api/db/repository/mongo/mongodb_client.go:79
- Renaming these methods removes the previous CreateEntity/ReadEntity/UpdateEntity/DeleteEntity APIs, but there are still non-test call sites in the repo that reference the old names (e.g., opengin/core-api/engine/graph_metadata_manager.go and opengin/core-api/cmd/server/service.go). As-is, the core-api build will fail due to missing methods. Either update all remaining call sites in this PR or keep backwards-compatible wrappers (deprecated) that forward the old method names to the new *Metadata methods.
func (repo *MongoRepository) CreateMetadata(ctx context.Context, entity *pb.Entity) (*mongo.InsertOneResult, error) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
opengin/core-api/db/repository/mongo/mongodb_client_test.go:214
- The test name/commentary in this section still refers to deleting an “entity”, but the repository call is now
DeleteMetadataand is specifically deleting the metadata document. Consider updating the wording to avoid implying the underlying graph/entity is deleted.
// Create entity
_, err = testRepo.CreateMetadata(testCtx, entity)
assert.NoError(t, err)
// Delete entity
result, err := testRepo.DeleteMetadata(testCtx, entityID)
assert.NoError(t, err)
assert.Equal(t, int64(1), result.DeletedCount)
// Verify entity is deleted
_, err = testRepo.ReadMetadata(testCtx, entityID)
assert.Error(t, err) // Should return an error since entity doesn't exist
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
opengin/core-api/db/repository/mongo/mongodb_client_test.go:214
- This test now exercises DeleteMetadata/ReadMetadata, but the test name and the doc comment block above still reference "DeleteEntity" and deleting an "entity". Please update the test name and the doc comment so they match the renamed API and describe that only metadata is being deleted from MongoDB.
// Delete metadata
result, err := testRepo.DeleteMetadata(testCtx, entityID)
assert.NoError(t, err)
assert.Equal(t, int64(1), result.DeletedCount)
// Verify metadata is deleted
_, err = testRepo.ReadMetadata(testCtx, entityID)
assert.Error(t, err) // Should return an error since metadata doesn't exist
|
Hi @prdai. One of your workflows is failing due to a deprecated key in the workflow file. I have now fixed that and merged it into main. Please rebase your code from main, I will then rerun your workflows. |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
5f979fc to
77e2bc9
Compare
|
hi @zaeema-n, i rebased with upstream main, let me know if there is anything else to be done, thanks! |
|
LGTM! Thank you for your contribution @prdai! |
summary
renames the crud operations for mongodb from using entity to metadata, as they were in reality updating the metadata instead of the entity it self, and updates any references that was using the functions that was renamed.
validation
fixes: #340