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

Added Insert One and Create Collection commands #16

Merged
merged 11 commits into from
Dec 23, 2022

Conversation

maheshrajamani
Copy link
Contributor

No description provided.

@maheshrajamani maheshrajamani marked this pull request as ready for review December 22, 2022 18:59
@maheshrajamani maheshrajamani requested a review from a team as a code owner December 22, 2022 18:59
@maheshrajamani maheshrajamani marked this pull request as draft December 22, 2022 19:00
@maheshrajamani maheshrajamani marked this pull request as ready for review December 22, 2022 21:06
# Conflicts:
#	src/main/java/io/stargate/sgv3/docsapi/exception/ErrorCode.java
#	src/main/java/io/stargate/sgv3/docsapi/service/shredding/Shredder.java
#	src/main/java/io/stargate/sgv3/docsapi/service/shredding/model/WritableShreddedDocument.java
@maheshrajamani
Copy link
Contributor Author

/gcbrun

@maheshrajamani maheshrajamani merged commit 036223c into main Dec 23, 2022
@maheshrajamani maheshrajamani deleted the create_and_insert branch December 23, 2022 03:47
Copy link
Contributor

@ivansenic ivansenic left a comment

Choose a reason for hiding this comment

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

I just realized after reviewing this that it's already merged..

I am quite shocked that there was an approving review on a PR with almost 1K code changes without any comment @tatu-at-datastax ? And I just pushed 27 of them.. This is not how we can work together, sorry but it's not point of review to get approval so it can be merged asap..

It's also not fair that I am kicking my ass out to review everything in details, it takes a lot of energy and time and I expect same from the whole team..

I expect that all of my comments are addressed in a followup PR.

pom.xml Show resolved Hide resolved
pom.xml Show resolved Hide resolved
pom.xml Show resolved Hide resolved
INSERTED_IDS;
INSERTED_IDS,
@JsonProperty("ok")
CREATE_COLLECTION;
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this not called OK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As per MongoDB documentation createCollection returns response as { "ok" : 1 }

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I meant why is enum not called OK 😄

Comment on lines +15 to +16
implementation = Object.class,
type = SchemaType.OBJECT)
Copy link
Contributor

Choose a reason for hiding this comment

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

not needed would be automatically worked out

.when()
.post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName)
.then()
.statusCode(200);
Copy link
Contributor

Choose a reason for hiding this comment

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

Everything returns 200, we must be more detailed:

body("status.insertedIds[0]", is("doc1"))

@@ -56,6 +83,29 @@ public void happyPath() {

@Nested
class InsertOne {
@Test
public void insertDocument() {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add insert multiple documents

Comment on lines +34 to +54
@Test
public final void createCollection() {
String json =
String.format(
"""
{
"createCollection": {
"name": "%s"
}
}
""",
collectionName);
given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
.contentType(ContentType.JSON)
.body(json)
.when()
.post(DatabaseResource.BASE_PATH, keyspaceId.asInternal())
.then()
.statusCode(200);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

what's this? if the collection does not exists it should be created by the insertOne.. This now makes test pass only with actually executing this first, which is not even guaranteed with this setup

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.

3 participants