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

WIP Feature/first api connection #13

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

dkraemerwork
Copy link
Contributor

please verify and pull it in man


@Bean
public SecurityWebFilterChain springSecurityFilterChain(ServerHttpSecurity http) {
http.authorizeExchange().anyExchange().permitAll();
Copy link
Member

@darivs darivs Dec 4, 2019

Choose a reason for hiding this comment

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

Ich glaube wenn du hier .csrf().disable() nach http.authorizeExchange() hängst, kannst du dir die @CrossOrigin Annotation im ReportBookletCreationService und somit auch im ganzen restlichen Code sparen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope funktioniert leider nicht.
.crsf().disable() und http.autohrizeExchange() bringen nichts.
Nur @crossorigin bringt was


@SpringBootApplication
@SpringBootApplication(exclude = { SecurityAutoConfiguration.class })
Copy link
Member

@darivs darivs Dec 4, 2019

Choose a reason for hiding this comment

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

Wieso musstest du dies excluden?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Das war einer der versuche den Network Error zu fixen.

@CrossOrigin(origins = "http://localhost:8081")
@RestController
@RequestMapping("/api")
public class ReportBookletCreationService {
Copy link
Member

Choose a reason for hiding this comment

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

Genau genommen wäre das ein Controller, in etwa ReportBookletController die Serviceschicht dient zu Operationen mit Repositories bzw als Zwischenschritt: Controller mit Restapi ReportBookletController -> ruft Service Methode auf aus ReportBookletService -> führt Repository oder ggf. andere Operation aus ReportBookletRepository.

Im Controller und Service haben wir dann verschiedene Apis & Methoden wie z.B. delete save get und würden dann nach Entitäten untergliedern, also ReportBookletController und -Service welchealle nötigen Operationen beinhalten

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Habe für die Änderung nicht so tief gedacht. ReportBookletCreationService sollte als Beispielklasse für "proof of concept" dienen für mich selber :'D
Aber danke für die Erläuterung :)


@CrossOrigin(origins = "http://localhost:8081")
@RestController
@RequestMapping("/api")
Copy link
Member

Choose a reason for hiding this comment

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

würde hier ("/api/booklet") oder so bevorziehen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wie schon erwähnt war dies als Proof of concept gedacht. Aber guter Vorschlag.

@RequestMapping("/api")
public class ReportBookletCreationService {

@RequestMapping( path = "/")
Copy link
Member

Choose a reason for hiding this comment

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

@GetMapping kannst du auch direkt schreiben, der Parameter path wird oben an den RequestMapping Pfad gehangen und kann hier auf jeden Fall weggelassen werden. Wenn wir oben ("/api/booklet") haben koennen wir z.B. zwei Apis ohne expliziten path schreiben, die eine mit @GetMapping annotiert und die andere mit @PostMapping, so wird dann einfach beim Aufruf von /api/booklet am Requesttype entschieden welche Methode aufgerufen wird, und wir koennen einfach get und post auf /api/booklet aufrufen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Das war mir nicht klar. Habe sehr viele unterschiedliche Beispiele online gesehen, aber nun weiß ich wie es besser zu nutzen ist.


@RequestMapping( path = "/")
public Object greeting(){
return "test";
Copy link
Member

Choose a reason for hiding this comment

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

Ach nice, muss das garkeine ResponseEntity sein?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Soweit ich weiß nicht.
ResponseEntity sind natürlich im zusammenspiel mit JPA besser, aber der Rückggabetyp kann vermutlich jeder Content-Type sein.

Copy link
Member

@darivs darivs left a comment

Choose a reason for hiding this comment

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

Nicer PR! :^) Wir könnten das Setzen der axios URL und der header noch in eine eigene .js outsourcen. Und allgemein sind feste Ports im Code immer not so nice, die holen wir besser später dann aus den environment variables. Aber nice, war das dein erster vollständiger backend-api-frontend flow?

@dkraemerwork
Copy link
Contributor Author

Ich hätte vielleicht aufräumen sollen bevor ich einen PR mache :D

@dkraemerwork
Copy link
Contributor Author

Bezüglich Axios und die neue .js, hatte ich auch schon im Hinterkopf. Für PoC hats so gereicht.
Ich habe die Base URL in der configure.js gesetzt, weil bei deinem Api Call für das Bild das objekt $axios verwendet wird wo der property Base URL localhost:8081/ zugewiesen ist.

@dkraemerwork dkraemerwork changed the title Feature/first api connection WIP Feature/first api connection Dec 4, 2019
@dkraemerwork
Copy link
Contributor Author

Bitte um erneutes feedback.

@darivs darivs self-requested a review December 19, 2019 12:32
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