14.09.2023 issue-52: insert chat id, token, token alias into database if token is valid #65

iColdLight · 2023-09-14T19:29:11Z

Added dependency for json web token in the pom.xml file
Added validateToken method in the TokenController class
Modified registerToken method to include validation of token in the TokenController class

PR-Codex overview

Focus of the PR:

This PR focuses on adding token validation and insertion functionality to the codebase.

Detailed summary:

Added TokenValidation class in io.blamer.hub.pg package.
TokenValidation implements the Tokens interface.
Added validateAndInsertToken method in TokenValidation to validate and insert tokens.
Added validateToken method in TokenValidation to validate tokens.
Added checkIfTokenExists method in TokenValidation to check if a token already exists in the database.
Added SQL query to check if a token exists in the checkIfTokenExists method.
Added TokenAlreadyExists exception in io.blamer.hub.exceptions package.
Added TokenInsertion class in io.blamer.hub.pg package to handle token insertion.
TokenInsertion implements the Tokens interface.
Added add method in TokenInsertion to add tokens.
Modified TokenController class in io.blamer.hub.controller package.
Modified the TokenController constructor to accept a Tokens object with @Qualifier("tokenValidation").
Modified the registerToken method in TokenController to use the tokens object to add tokens.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

zoeself · 2023-09-14T19:29:14Z

@iColdLight thank you for your Pull Request. I'll assign someone to review it soon.

If this PR solves a todo from the code, please don't forget to remove it.

zoeself · 2023-09-14T19:31:40Z

@h1alexbel please review this Pull Request. Deadline (when it should be merged or closed) is 2023-09-17T19:18:59.313614.

You should check if the requirements have been implemented (partially or in full), if there are unit tests covering the changes and if the CI build passes. Feel free to reject the PR or ask for changes if it's too big or not clear enough.

Estimation here is 30 minutes, that's how much you will be paid. You will be paid even if this PR gets rejected.

h1alexbel

@iColdLight the ticket was to insert chat id, token, token alias and other stuff (can be just a placeholder for now) if the provided token is valid.
And this token is not JWT, it's a just simple GitHub token, simple string

WDYT?

h1alexbel · 2023-09-15T13:10:28Z

pom.xml

@@ -159,6 +160,11 @@ SOFTWARE.
      <artifactId>reactor-test</artifactId>
      <scope>test</scope>
    </dependency>
+    <dependency>


@iColdLight sort your dependencies, compile/provided scope first,
and only then test one

h1alexbel · 2023-09-15T13:11:01Z

src/main/java/io/blamer/hub/controller/TokenController.java

-  Mono<Void> registerToken(@RequestBody final RequestToken request)
-    throws Exception {
-    return this.tokens.add(new TokenToAdd(request).value());
+  Mono<Void> registerToken(@RequestBody final RequestToken request) throws Exception {


@iColdLight I think, method should be public

h1alexbel · 2023-09-15T13:12:15Z

src/main/java/io/blamer/hub/controller/TokenController.java

-    throws Exception {
-    return this.tokens.add(new TokenToAdd(request).value());
+  Mono<Void> registerToken(@RequestBody final RequestToken request) throws Exception {
+    String token = request.getValue();


@iColdLight make all your variables final

h1alexbel · 2023-09-15T13:13:01Z

src/main/java/io/blamer/hub/controller/TokenController.java

+  Mono<Void> registerToken(@RequestBody final RequestToken request) throws Exception {
+    String token = request.getValue();
+    String secretKey = "your-secret-key";
+


@iColdLight empty lines mean code smell, read this

zoeself · 2023-09-16T00:41:48Z

@h1alexbel Don't forget to close this ticket before the deadline (2023-09-17T19:18:59). You are past the first half of the allowed period.

iColdLight · 2023-09-17T07:01:42Z

Does it mean that I need to work on creating JWT TokenProvider, JWT TokenFIlter, JWT User, etc.?

h1alexbel · 2023-09-17T09:59:40Z

@iColdLight we don't need JWTs. We are dealing with simple text token from GitHub

And this token is not JWT, it's a just simple GitHub token, simple string

zoeself · 2023-09-17T19:42:00Z

@h1alexbel Looks like you've missed the task deadline (2023-09-17T19:18:59). You are now resigned from this task.

Please stop working on it, you will not be paid. I will assign it to someone else soon.

zoeself · 2023-09-17T19:42:21Z

@h1alexbel I couldn't find any assignee for this task. This is either because there are no contributors with role REV available or because the project does not have enough funds.

Please, make sure there is at least one available contributor with the required role and the project can afford to pay them.

iColdLight · 2023-09-17T20:24:05Z

Hi!
I made new changes. I wonder, if you can see them.
And sorry for 3 commits in a row...
made slight changes in the last two.

l3r8yJ · 2023-09-18T16:43:58Z

@iColdLight it's okay with commits, but you can read this

l3r8yJ · 2023-09-18T16:44:09Z

@h1alexbel reminder

h1alexbel

@iColdLight check my comments, please

h1alexbel · 2023-09-19T06:26:34Z

src/main/java/io/blamer/hub/controller/TokenController.java

-   * Tokens.
-   */
-  private final Tokens tokens;
+    /**


@iColdLight don't change the layout, please

h1alexbel · 2023-09-19T06:27:49Z

src/main/java/io/blamer/hub/exceptions/TokenAlreadyExists.java

@@ -0,0 +1,7 @@
+package io.blamer.hub.exceptions;
+
+public class TokenAlreadyExists extends RuntimeException{


@iColdLight class can be final

h1alexbel · 2023-09-19T06:28:51Z

src/main/java/io/blamer/hub/pg/TokenValidated.java

+import org.springframework.stereotype.Component;
+import reactor.core.publisher.Mono;
+@Component
+public class TokenValidated implements Tokens {


@iColdLight It's better to name this class just like Validated, since your interface already saying that this is Tokens

I didn't name my class Validated because we already have a class with that name

h1alexbel · 2023-09-19T06:32:04Z

src/main/java/io/blamer/hub/pg/TokenValidated.java

+
+    @Override
+    public Mono<Void> add(final Token token) {
+        return validateToken(token)


@iColdLight
This logic can be decorated, presented as at least 2 classes: first where you just insert token, and the second one where you check token validity and integrity, and then insert into database (reuse first object)
What do you think?

Added TokenInsertion class Reworked TokenValidation class Made TokenAlreadyExists final

l3r8yJ · 2023-09-24T14:41:51Z

/codex Could there be a problem here in terms of non-blocking interaction?

pr-codex · 2023-09-24T14:42:00Z

Could there be a problem here in terms of non-blocking interaction?

@l3r8yJ Yes, there could be a problem with non-blocking interaction in the code. The TokenValidation class implements the Tokens interface and overrides the add method. Inside the add method, there are multiple database interactions (validateToken, checkIfTokenExists, and this.tokenInsertion.add) that return Mono objects, indicating asynchronous operations.

However, the code does not utilize reactive operators like flatMap, map, or zip to chain these operations together and ensure non-blocking execution. Instead, it directly calls these methods synchronously, which may lead to blocking behavior and potential performance issues.

To ensure non-blocking interaction, the code should use reactive operators to compose the asynchronous operations and handle the results asynchronously.

l3r8yJ · 2023-09-24T14:43:24Z

@iColdLight take note of this comment

Could there be a problem here in terms of non-blocking interaction?

@l3r8yJ Yes, there could be a problem with non-blocking interaction in the code. The TokenValidation class implements the Tokens interface and overrides the add method. Inside the add method, there are multiple database interactions (validateToken, checkIfTokenExists, and this.tokenInsertion.add) that return Mono objects, indicating asynchronous operations.

However, the code does not utilize reactive operators like flatMap, map, or zip to chain these operations together and ensure non-blocking execution. Instead, it directly calls these methods synchronously, which may lead to blocking behavior and potential performance issues.

To ensure non-blocking interaction, the code should use reactive operators to compose the asynchronous operations and handle the results asynchronously.

it's an AI response, could be wrong

Reworked validateAndInsertToken method in TokenValidation class

iColdLight · 2023-09-27T19:58:41Z

I made a change to my TokenValidation class to ensure non-blocking interaction. May you take a look?

h1alexbel

@iColdLight core logic is fine, now is needed to pass the merge pipeline

h1alexbel · 2023-09-28T04:19:33Z

@l3r8yJ take a look, please

issue-52 14.09.2023

c4a6bd2

zoeself assigned h1alexbel Sep 14, 2023

zoeself added the @h1alexbel label Sep 14, 2023

l3r8yJ requested a review from h1alexbel September 14, 2023 20:06

h1alexbel requested changes Sep 15, 2023

View reviewed changes

zoeself unassigned h1alexbel Sep 17, 2023

zoeself removed the @h1alexbel label Sep 17, 2023

iColdLight added 3 commits September 17, 2023 23:03

issue-52 17.09.2023

6c14d89

issue-52 17.09.2023-2

e7d0cb9

issue-52 17.09.2023-3

1e951bd

iColdLight requested a review from h1alexbel September 18, 2023 15:25

h1alexbel requested changes Sep 19, 2023

View reviewed changes

issue-52 24.09.2023

804b06f

Added TokenInsertion class Reworked TokenValidation class Made TokenAlreadyExists final

iColdLight requested a review from h1alexbel September 24, 2023 14:37

issue-52 27.09.2023

870fb0e

Reworked validateAndInsertToken method in TokenValidation class

h1alexbel approved these changes Sep 28, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

14.09.2023 issue-52: insert chat id, token, token alias into database if token is valid #65

14.09.2023 issue-52: insert chat id, token, token alias into database if token is valid #65

iColdLight commented Sep 14, 2023 •

edited by pr-codex bot

Loading

zoeself commented Sep 14, 2023

zoeself commented Sep 14, 2023

h1alexbel left a comment

h1alexbel Sep 15, 2023

h1alexbel Sep 15, 2023

h1alexbel Sep 15, 2023

h1alexbel Sep 15, 2023

zoeself commented Sep 16, 2023

iColdLight commented Sep 17, 2023

h1alexbel commented Sep 17, 2023

zoeself commented Sep 17, 2023

zoeself commented Sep 17, 2023

iColdLight commented Sep 17, 2023

l3r8yJ commented Sep 18, 2023

l3r8yJ commented Sep 18, 2023

h1alexbel left a comment

h1alexbel Sep 19, 2023

h1alexbel Sep 19, 2023

h1alexbel Sep 19, 2023

iColdLight Sep 24, 2023

h1alexbel Sep 19, 2023

l3r8yJ commented Sep 24, 2023

pr-codex bot commented Sep 24, 2023

l3r8yJ commented Sep 24, 2023 •

edited

Loading

iColdLight commented Sep 27, 2023

h1alexbel left a comment

h1alexbel commented Sep 28, 2023

		@@ -0,0 +1,7 @@
		package io.blamer.hub.exceptions;

		public class TokenAlreadyExists extends RuntimeException{

14.09.2023 issue-52: insert chat id, token, token alias into database if token is valid #65

Are you sure you want to change the base?

14.09.2023 issue-52: insert chat id, token, token alias into database if token is valid #65

Conversation

iColdLight commented Sep 14, 2023 • edited by pr-codex bot Loading

PR-Codex overview

Focus of the PR:

Detailed summary:

zoeself commented Sep 14, 2023

zoeself commented Sep 14, 2023

h1alexbel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zoeself commented Sep 16, 2023

iColdLight commented Sep 17, 2023

h1alexbel commented Sep 17, 2023

zoeself commented Sep 17, 2023

zoeself commented Sep 17, 2023

iColdLight commented Sep 17, 2023

l3r8yJ commented Sep 18, 2023

l3r8yJ commented Sep 18, 2023

h1alexbel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

l3r8yJ commented Sep 24, 2023

pr-codex bot commented Sep 24, 2023

l3r8yJ commented Sep 24, 2023 • edited Loading

iColdLight commented Sep 27, 2023

h1alexbel left a comment

Choose a reason for hiding this comment

h1alexbel commented Sep 28, 2023

iColdLight commented Sep 14, 2023 •

edited by pr-codex bot

Loading

l3r8yJ commented Sep 24, 2023 •

edited

Loading