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

(dsl): Support regexp query #302

Merged
merged 3 commits into from
Aug 21, 2023

Conversation

milicns
Copy link
Contributor

@milicns milicns commented Aug 15, 2023

Part of #91

res <- Executor
.execute(ElasticRequest.search(firstSearchIndex, query))
.documentAs[TestDocument]
} yield assert(res)(!Assertion.contains(firstDocument))
Copy link
Collaborator

Choose a reason for hiding this comment

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

It does not make sense to me. Should it maybe be assert(res)(Assertion.contains(firstDocument))?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It probably fails.

Copy link
Contributor Author

@milicns milicns Aug 16, 2023

Choose a reason for hiding this comment

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

Agreed to just change the name of the test.

Comment on lines 1081 to 1108
test("search for a document using regexp query with case insensitive") {
checkOnce(genDocumentId, genTestDocument, genDocumentId, genTestDocument) {
(firstDocumentId, firstDocument, secondDocumentId, secondDocument) =>
for {
_ <- Executor.execute(ElasticRequest.deleteByQuery(firstSearchIndex, matchAll))
_ <- Executor.execute(
ElasticRequest.upsert[TestDocument](firstSearchIndex, firstDocumentId, firstDocument)
)
_ <- Executor.execute(
ElasticRequest
.upsert[TestDocument](firstSearchIndex, secondDocumentId, secondDocument)
.refreshTrue
)
query = ElasticQuery
.regexp(
field = TestDocument.stringField,
value = s"${firstDocument.stringField.take(1)}.*${firstDocument.stringField.takeRight(1)}"
)
.caseInsensitiveTrue
res <- Executor
.execute(ElasticRequest.search(firstSearchIndex, query))
.documentAs[TestDocument]
} yield (assert(res)(Assertion.contains(firstDocument)) && assert(res)(!Assertion.contains(secondDocument)))
}
} @@ around(
Executor.execute(ElasticRequest.createIndex(firstSearchIndex)),
Executor.execute(ElasticRequest.deleteIndex(firstSearchIndex)).orDie
),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can remove this test.

Copy link
Contributor Author

@milicns milicns Aug 16, 2023

Choose a reason for hiding this comment

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

Agreed to just change the name of the previous test.

* [[zio.elasticsearch.query.RegexpQuery]] returns documents that contain terms matching a regular expression.
*
* @param field
* the field for which query is specified for
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* the field for which query is specified for
* the type-safe field for which query is specified for

test("regexp") {
val query = regexp("stringField", "t.*st")
val queryTs = regexp(TestDocument.stringField, "t.*st")
val queryWithCaseInsensitive = regexp("stringField", "t.*st").caseInsensitiveTrue
Copy link
Collaborator

@dbulaja98 dbulaja98 Aug 16, 2023

Choose a reason for hiding this comment

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

You don't need this one. (line 1171)

val query = regexp("stringField", "t.*st")
val queryTs = regexp(TestDocument.stringField, "t.*st")
val queryWithCaseInsensitive = regexp("stringField", "t.*st").caseInsensitiveTrue
val queryTsWithCaseInsensitive = regexp(TestDocument.stringField, "t.*st").caseInsensitiveTrue
Copy link
Collaborator

@dbulaja98 dbulaja98 Aug 16, 2023

Choose a reason for hiding this comment

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

Rename this one to queryWithCaseInsensitive. (line 1171)

Copy link
Collaborator

Choose a reason for hiding this comment

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

And add another one val queryWithSuffix, where you can do something like this: TestDocument.stringField.raw.

@@ -2772,6 +2783,40 @@ object ElasticQuerySpec extends ZIOSpecDefault {
assert(queryMixedBoundsWithBoost.toJson(fieldPath = None))(equalTo(expectedMixedBoundsWithBoost.toJson)) &&
assert(queryWithFormat.toJson(fieldPath = None))(equalTo(expectedWithFormat.toJson))
},
test("regexp") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do the same as above here.

) &&
assert(queryWithSuffix)(
equalTo(Regexp[TestDocument](field = "stringField.raw", value = "t.*st", caseInsensitive = None))
)
Copy link
Contributor Author

@milicns milicns Aug 16, 2023

Choose a reason for hiding this comment

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

Sbt prepare divide these asserts in the new lines

@milicns milicns requested a review from dbulaja98 August 16, 2023 13:17
@dbulaja98 dbulaja98 merged commit 1ac7c4e into lambdaworks:main Aug 21, 2023
13 checks passed
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

Successfully merging this pull request may close these issues.

2 participants