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

feature/IL-77/Add-OpenAPI-for-cart-package #100

Merged
merged 1 commit into from
Sep 17, 2023

Conversation

shpali4
Copy link
Contributor

@shpali4 shpali4 commented Sep 17, 2023

  1. Developer's full name:
    Shamardin Vladislav
  2. Link to Documentation:
    (will be updated after merge)
  3. Changes:
    3.1 CartEndpoint was updated (implemented CartsApi, removed @Valid annotations from methods)
    3.2 cart-openapi.yaml was added
    3.3 pom.xml was updated
  4. Test data
    Import cart-openapi.yaml in Postman, after authorization all responses 200 OK
  5. Conclusion:
    5.1 Project was built with Apache Maven successfully.
    5.2 Project was tested manually with Postman successfully.

@@ -30,14 +31,14 @@
@RequiredArgsConstructor
@Validated
@RequestMapping(value = CartEndpoint.CART_URL)
public class CartEndpoint {
public class CartEndpoint implements CartsApi {

public static final String CART_URL = "/api/v1/cart";

private final CartApi cartApi;

@PostMapping(value = "/items")
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to @OverRide the method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are overrided automaticly, isn't it?

Copy link
Owner

@Sunagatov Sunagatov Sep 17, 2023

Choose a reason for hiding this comment

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

using this annotation is like a best practice
it is really good idea to add it

@@ -57,7 +58,7 @@ public ResponseEntity<ShoppingSessionDto> getShoppingSession(@AuthenticationPrin
}

@PatchMapping(value = "/items")
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to @OverRide the method

@@ -69,7 +70,7 @@ public ResponseEntity<ShoppingSessionDto> updateProductsQuantityInShoppingSessio
}

@DeleteMapping(value = "/items")
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to @OverRide the method

@@ -0,0 +1,192 @@
openapi: "3.0.3"

info:
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to write in a common style as in Zufar's example in product-openapi.yaml

info:
title: "Online-Store API"
version: "1.0.0"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

schema:
$ref: "#/components/schemas/ShoppingSessionDto"

delete:
Copy link
Contributor

Choose a reason for hiding this comment

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

better to write path as in a common style

Copy link
Owner

@Sunagatov Sunagatov left a comment

Choose a reason for hiding this comment

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

Reviewed


public static final String CART_URL = "/api/v1/cart";

private final CartApi cartApi;

@PostMapping(value = "/items")
public ResponseEntity<ShoppingSessionDto> addNewItemToShoppingSession(@RequestBody @Valid final AddNewItemsToShoppingSessionRequest request) {
public ResponseEntity<ShoppingSessionDto> addNewItemToShoppingSession(@RequestBody final AddNewItemsToShoppingSessionRequest request) {
Copy link
Owner

Choose a reason for hiding this comment

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

Why did you delete ‘@Valid'?
Rollback it, please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With this annotation we have a trouble in case when we try to use our API not in IDE (i use Postman): Hibernate validator HV000151 "A method overriding another method must not alter the parameter constraint configuration").
In our generated interface we have the same annotation, its incorrect.
More info: https://forum.hibernate.org/viewtopic.php?p=2469903

@@ -30,14 +31,14 @@
@RequiredArgsConstructor
@Validated
@RequestMapping(value = CartEndpoint.CART_URL)
public class CartEndpoint {
public class CartEndpoint implements CartsApi {
Copy link
Owner

Choose a reason for hiding this comment

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

CartApi sounds better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are already have CartApi in this class (import com.zufar.onlinestore.cart.api.CartApi). Is it ok to use same names?

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, there is no problem

@@ -57,7 +58,7 @@ public ResponseEntity<ShoppingSessionDto> getShoppingSession(@AuthenticationPrin
}

@PatchMapping(value = "/items")
public ResponseEntity<ShoppingSessionDto> updateProductsQuantityInShoppingSessionItem(@RequestBody @Valid final UpdateProductsQuantityInShoppingSessionItemRequest request) {
Copy link
Owner

Choose a reason for hiding this comment

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

Rollback ‘@Valid' annotation, please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same

@@ -69,7 +70,7 @@ public ResponseEntity<ShoppingSessionDto> updateProductsQuantityInShoppingSessio
}

@DeleteMapping(value = "/items")
public ResponseEntity<ShoppingSessionDto> deleteItemsFromShoppingSession(@RequestBody @Valid final DeleteItemsFromShoppingSessionRequest request) {
Copy link
Owner

Choose a reason for hiding this comment

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

Rollback ‘@Valid' annotation, please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same

- url: "http://localhost:8083"

tags:
- name: "Carts"
Copy link
Owner

Choose a reason for hiding this comment

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

Cart is better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same

@Sunagatov Sunagatov merged commit 1e6c954 into development Sep 17, 2023
2 of 3 checks passed
@Sunagatov Sunagatov deleted the feature/IL-77/Add-OpenAPI-for-cart-package branch May 10, 2024 21:07
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