Skip to content
This repository has been archived by the owner on Jan 8, 2025. It is now read-only.

Implement CollectionName enum to store db collection names #935

Merged
merged 5 commits into from
Apr 10, 2024

Conversation

tcoratger
Copy link
Contributor

Time spent on this PR: 20 min

Pull Request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build-related changes
  • Documentation content changes
  • Testing

What is the new behavior?

To avoid any typos or errors when writing the names of the collections in the Mongo DB database, a CollectionName enum is created and it contains an as_str method in its implementation which allows you to find the name of all collections in a Rust idiomatic way.

Does this introduce a breaking change?

  • Yes
  • No

Copy link
Contributor

@greged93 greged93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea, but what do you think of the following:

  • Add a trait which allows to convert a type to string (as a static function, not a method).
   pub trait ToStringType {
      fn to_string() -> String;
   }
  • Implement this on the StoredXXX types.
  • Update the Mongo wrapper to take
pub async fn get_one<T>(
        &self,
        filter: impl Into<Option<Document>>,
        sort: impl Into<Option<Document>>,
    ) -> DatabaseResult<Option<T>>
    where
        T: DeserializeOwned + ToStringType + Unpin + Send + Sync,

Then we can just do in the body of the function

let find_one_option = FindOneOptions::builder().sort(sort).build();
let collection = self.0.collection::<T>(T::to_string());
let result = collection.find_one(filter, find_one_option).await?;
Ok(result)

@tcoratger
Copy link
Contributor Author

I like this idea, but what do you think of the following:

  • Add a trait which allows to convert a type to string (as a static function, not a method).
   pub trait ToStringType {
      fn to_string() -> String;
   }
  • Implement this on the StoredXXX types.
  • Update the Mongo wrapper to take
pub async fn get_one<T>(
        &self,
        filter: impl Into<Option<Document>>,
        sort: impl Into<Option<Document>>,
    ) -> DatabaseResult<Option<T>>
    where
        T: DeserializeOwned + ToStringType + Unpin + Send + Sync,

Then we can just do in the body of the function

let find_one_option = FindOneOptions::builder().sort(sort).build();
let collection = self.0.collection::<T>(T::to_string());
let result = collection.find_one(filter, find_one_option).await?;
Ok(result)

Nice, I just took up this idea almost completely. I just called the trait a little differently so as not to cause confusion with the many ToString traits or other similar names that are found everywhere in a lots of codebases.

@greged93 greged93 added this pull request to the merge queue Apr 10, 2024
Merged via the queue into kkrt-labs:main with commit cd8d612 Apr 10, 2024
5 checks passed
anukkrit149 pushed a commit to karnotxyz/kakarot-rpc that referenced this pull request Aug 9, 2024
<!--- Please provide a general summary of your changes in the title
above -->

<!-- Give an estimate of the time you spent on this PR in terms of work
days.
Did you spend 0.5 days on this PR or rather 2 days?  -->

Time spent on this PR:

## Pull request type

<!-- Please try to limit your pull request to one type,
submit multiple pull requests if needed. -->

Please check the type of change your PR introduces:

- [ ] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe):

## What is the current behavior?

<!-- Please describe the current behavior that you are modifying,
or link to a relevant issue. -->

Resolves #<Issue number>

## What is the new behavior?

<!-- Please describe the behavior or changes that are being added by
this PR. -->

-
-
-

<!-- Reviewable:start -->
- - -
This change is [<img src="https://reviewable.io/review_button.svg"
height="34" align="absmiddle"
alt="Reviewable"/>](https://reviewable.io/reviews/kkrt-labs/kakarot/935)
<!-- Reviewable:end -->
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants