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

New structure and tests #5

Merged
merged 11 commits into from
Jun 9, 2024
Merged

New structure and tests #5

merged 11 commits into from
Jun 9, 2024

Conversation

MacStellar
Copy link
Owner

Fixade ny struktur, fixade till testen, ta till lite nya test. Gick inte på djupet och putsade till koden supermycket utan tänker att det är bättre att du tar en titt först och ser att jag är på rätt väg innan jag investerar för mycket :)

…ka returna nya JSON grejer), PPID kopplar till refreshtoken och p2pSession istället för cookies, allt funkar förutom return på api:er som jag ska fixa senare.
…ed textomvanling från API:s return till testets kontroll, fixar detta senare.
@MacStellar
Copy link
Owner Author

Fixade till backend. Tänker att den får vara som den är utan större uppgraderingar tills inlämningen, men om du hittar något fel eller så så fixar jag till det! Ska köra static code analysis sen också. Gäller bara backendkoden, frontend får jag försöka hinna senare :)

Comment on lines +435 to +437
sessionDB.findById(p2pSessionId).orElseThrow {
P2PSessionNotFound()
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Behöver vi kolla att sessionen är i rätt state?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Den funktionen är egentligen bara till för att return:a statet som sessionen är i, så tror inte det. Eller ska man kolla så att den inte är null eller något sånt?

class P2PApiTest {
@Autowired
lateinit var testRestTemplate: TestRestTemplate

Copy link
Collaborator

Choose a reason for hiding this comment

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

Kan vara bra att ha några test av allmänna säkerheten på toppnivån

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ok, tänker du typ code injection och sånt? Är inte riktigt säker på vilka jag ska ha med

@johanneslundsten
Copy link
Collaborator

Bra jobbat!

Blev lite kommentarer, tror inte du behöver köra igenom alla.
Men vi kanske kan ta en titt tillsammans på man-in-the-middle då det är en rätt central grej.
Sen vore det skoj med frontend, men det kanske är tajt

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…er.kt

Co-authored-by: johanneslundsten <[email protected]>
@MacStellar MacStellar merged commit 1a8a4aa into main Jun 9, 2024
1 check 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.

None yet

2 participants