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

Implement InSearchOperatorDsl #94

Merged
merged 14 commits into from
Oct 31, 2023

Conversation

cranemont
Copy link
Contributor

resolves #10

@wo0dev wo0dev assigned wo0dev and cranemont and unassigned wo0dev Oct 11, 2023
@wo0dev wo0dev requested review from inflab-int and jbl428 and removed request for inflab-int October 11, 2023 09:47
}

/**
* The score assigned to matching search term results. Use one of the following options to modify the score:
Copy link
Contributor

Choose a reason for hiding this comment

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

Please insert a new line after the period.

Suggested change
* The score assigned to matching search term results. Use one of the following options to modify the score:
* The score assigned to matching search term results.
* Use one of the following options to modify the score:

* - constant: replace the result score with the given number.
* - function: replace the result score using the given expression.
*
* @param scoreConfiguration The configuration block for [ScoreSearchOptionDsl]
Copy link
Contributor

Choose a reason for hiding this comment

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

Each sentence must be ended with period.

Suggested change
* @param scoreConfiguration The configuration block for [ScoreSearchOptionDsl]
* @param scoreConfiguration The configuration block for [ScoreSearchOptionDsl].

Comment on lines 44 to 70
/**
* Value or values to search.
* Value can be either a single value or an array of values of only one of the supported BSON types and can't be a mix of different types.
*
* @param value Value to search.
*/
fun value(value: Boolean) {
document["value"] = value
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Since there is a method that has vararg boolean argument,
we can remove this one. (same as Temporal, Number, ObjectId)

*
* @param value Values to search.
*/
fun value(vararg value: Boolean) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to have a method of iterable arg version.

    /**
     * Value or values to search.
     * Value can be either a single value or an array of values of only one of the supported BSON types and can't be a mix of different types.
     *
     * @param value Values to search.
     */
    fun value(value: Iterable<Boolean>) {
        document["value"] = value
    }

* @param path Indexed field to search.
* @see <a href="https://www.mongodb.com/docs/atlas/atlas-search/path-construction/#std-label-ref-path">Path Construction</a>
*/
fun path(path: String) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we can have KProperty version of overload method.
this provide a user type-safety.

    fun path(path: KProperty<Boolean?>) {
        document["path"] = path
    }

    fun path(path: KProperty<Iterable<Boolean?>?>) {
        document["path"] = path
    }

compound {
must {
text {
path("name")
Copy link
Contributor

Choose a reason for hiding this comment

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

If we have Kproperty path method. we can replace this line

path(Customers::name)

@cranemont
Copy link
Contributor Author

@jbl428 Applied comments! Thank you for the kind guide, and I'm sorry for the late change.

Copy link
Contributor

@jbl428 jbl428 left a comment

Choose a reason for hiding this comment

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

Thank you for contribution.
I will handle the reviews after PR is merged :)

}
}

// TODO: add $limit stage
Copy link
Contributor

Choose a reason for hiding this comment

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

limit stage is now implemented, so we can use limit function

* @param path Indexed field to search.
* @see <a href="https://www.mongodb.com/docs/atlas/atlas-search/path-construction/#std-label-ref-path">Path Construction</a>
*/
fun path(path: KProperty<*>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that the param type should be KProperty<String>

@jbl428 jbl428 merged commit e79efe9 into inflearn:main Oct 31, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement InSearchOperatorDsl
3 participants