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

Change api generation #667

Merged
merged 4 commits into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ repos:
hooks:
- id: check-merge-conflict
- id: end-of-file-fixer
exclude: ^frontend2/src/utils/types
- id: mixed-line-ending
args: ["--fix=lf"]
exclude: ^frontend2/src/utils/types
- id: trailing-whitespace
exclude: ^frontend2/src/utils/types

- repo: https://github.com/dnephin/pre-commit-golang
rev: v0.5.1
Expand Down
4 changes: 4 additions & 0 deletions backend/siarnaq/api/compete/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -467,3 +467,7 @@ def save(self, *args, **kwargs):
class HistoricalRatingSerializer(serializers.Serializer):
rating = RatingField()
timestamp = serializers.DateTimeField()


class EmptySerializer(serializers.Serializer):
pass
15 changes: 12 additions & 3 deletions backend/siarnaq/api/compete/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
)
from siarnaq.api.compete.permissions import HasTeamSubmission
from siarnaq.api.compete.serializers import (
EmptySerializer,
HistoricalRatingSerializer,
MatchReportSerializer,
MatchSerializer,
Expand Down Expand Up @@ -149,8 +150,16 @@ def create(self, request, *, episode_id):
headers=headers,
)

@extend_schema(
# https://drf-spectacular.readthedocs.io/en/latest/faq.html#i-m-using-action-detail-false-but-the-response-schema-is-not-a-list
responses=TournamentSubmissionSerializer(many=True)
Copy link
Member Author

Choose a reason for hiding this comment

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

spectacular wasn't recognizing this as a list of tournament submissions, so I added many=True. With many=True, spectacular thought it was a paginated list, so I had to set pagination_class=None in the action decorator below

)
@action(
detail=False, methods=["get"], serializer_class=TournamentSubmissionSerializer
detail=False,
methods=["get"],
serializer_class=TournamentSubmissionSerializer,
# needed so that the generated schema is not paginated
pagination_class=None,
)
def tournament(self, request, *, episode_id):
"""Retrieve the submissions used in tournaments by the current team.."""
Expand Down Expand Up @@ -409,7 +418,7 @@ def report(self, request, pk=None, *, episode_id):
detail=True,
methods=["post"],
permission_classes=(IsAdminUser,),
serializer_class=None,
serializer_class=EmptySerializer,
Copy link
Member Author

Choose a reason for hiding this comment

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

Spectacular errors if the serializer class is None.

throttle_classes=(),
)
def rating_update(self, request, pk=None, *, episode_id):
Expand All @@ -424,7 +433,7 @@ def rating_update(self, request, pk=None, *, episode_id):
detail=True,
methods=["post"],
permission_classes=(IsAdminUser,),
serializer_class=None,
serializer_class=EmptySerializer,
throttle_classes=(),
)
def publish_public_bracket(self, request, pk=None, *, episode_id):
Expand Down
6 changes: 5 additions & 1 deletion backend/siarnaq/api/episodes/serializers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from drf_spectacular.types import OpenApiTypes
from drf_spectacular.utils import extend_schema_field
from drf_spectacular.utils import extend_schema_field, extend_schema_serializer
from rest_framework import serializers

from siarnaq.api.episodes.models import (
Expand Down Expand Up @@ -48,6 +48,10 @@ def get_frozen(self, obj):
return obj.frozen()


@extend_schema_serializer(
# workaround for https://github.com/OpenAPITools/openapi-generator/issues/9289
component_name="GameMap"
)
class MapSerializer(serializers.ModelSerializer):
class Meta:
model = Map
Expand Down
4 changes: 4 additions & 0 deletions backend/siarnaq/api/teams/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,7 @@ class UserPassedSerializer(serializers.Serializer):

class TeamReportSerializer(serializers.Serializer):
report = serializers.FileField(write_only=True)


class TeamLeaveSerializer(serializers.Serializer):
pass
3 changes: 2 additions & 1 deletion backend/siarnaq/api/teams/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
TeamAvatarSerializer,
TeamCreateSerializer,
TeamJoinSerializer,
TeamLeaveSerializer,
TeamPrivateSerializer,
TeamPublicSerializer,
TeamReportSerializer,
Expand Down Expand Up @@ -114,7 +115,7 @@ def me(self, request, *, episode_id):
@action(
detail=False,
methods=["post"],
serializer_class=None,
serializer_class=TeamLeaveSerializer,
permission_classes=(IsAuthenticated, IsEpisodeAvailable),
)
def leave(self, request, *, episode_id):
Expand Down
8 changes: 8 additions & 0 deletions backend/siarnaq/api/user/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ def me(self, request):
serializer.save()
return Response(serializer.data)

@extend_schema(
responses={
status.HTTP_200_OK: {
"type": "object",
"additionalProperties": {"$ref": "#/components/schemas/TeamPublic"},
}
Comment on lines +80 to +83
Copy link
Member Author

Choose a reason for hiding this comment

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

this schema dict describes the response as an object with arbitrary keys with each value conforming to the schema TeamPublic (which is generated from TeamPublicSerializer)

}
)
@action(
detail=True,
permission_classes=(AllowAny,),
Expand Down
15 changes: 15 additions & 0 deletions backend/siarnaq/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -510,3 +510,18 @@ def dropper(logger, method_name, event_dict):
raise structlog.DropEvent

structlog.configure(processors=[dropper])

SPECTACULAR_SETTINGS = {
# Split components into request and response parts where appropriate
# This setting is highly recommended to achieve the most accurate API
# description, however it comes at the cost of having more components.
"COMPONENT_SPLIT_REQUEST": True,
Copy link
Member Author

Choose a reason for hiding this comment

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

With this, the requests that we pass into API functiosn do not need to contain readonly fields anymore (e.g. no more passing in id:-1 to createuser)

Copy link
Contributor

Choose a reason for hiding this comment

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

🎉🎉

# Controls which authentication methods are exposed in the schema. If not None,
# will hide authentication classes that are not contained in the whitelist.
# Use full import paths like
# ['rest_framework.authentication.TokenAuthentication', ...].
# Empty list ([]) will hide all authentication methods. The default None shows all.
"AUTHENTICATION_WHITELIST": [
"rest_framework_simplejwt.authentication.JWTAuthentication"
Copy link
Member Author

Choose a reason for hiding this comment

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

This gets rid of a lot of the warnings from spectacular that ask us to make an OpenApiAuthenticationExtension for GoogleCloudAuthentication. We only use JWT authentication from the frontend, so we only need that in our schema.
Below is the warning that this line gets rid of:

Warning #0: MatchViewSet: could not resolve authenticator <class 'siarnaq.api.user.authentication.GoogleCloudAuthentication'>. There was no OpenApiAuthenticationExtension registered for that class. Try creating one by subclassing it. Ignoring for now.

],
}
1 change: 0 additions & 1 deletion frontend2/.prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
/node_modules
/.pnp
.pnp.js
/types

# production
/build
Expand Down
7 changes: 5 additions & 2 deletions frontend2/generate_types.bat
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@ call conda activate galaxy
echo "navigate to galaxy folder"
cd ..
echo "Generate OpenAPI 3.0 backend schema from siarnaq"
python backend/manage.py spectacular --file schema.yml
python backend/manage.py spectacular --file schema.yml --validate

:: TODO: delete the types folder before regenerating it.

:: echo "Download openapitools"
:: npm install @openapitools/openapi-generator-cli -g
:: if you have a java error, just install java 8 and reload terminal
echo "Generate typescript types from schema"
npx @openapitools/openapi-generator-cli generate -i schema.yml -o frontend2/src/utils/types -g typescript-jquery --additional-properties=modelPropertyNaming=camelCase --additional-properties=disallowAdditionalPropertiesIfNotPresent=false
npx @openapitools/openapi-generator-cli generate -i schema.yml -o frontend2/src/utils/types -g typescript-fetch --additional-properties=modelPropertyNaming=original --additional-properties=disallowAdditionalPropertiesIfNotPresent=false --additional-properties=stringEnums=true
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 original vs camelcase? does original just follow what you have
sorry if i missed slack

Copy link
Member Author

Choose a reason for hiding this comment

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

original follows what the backend uses (aka snake_case). if the schema is generated in camelCase it doesn't work b/c the backend expects snake_case

22 changes: 0 additions & 22 deletions frontend2/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions frontend2/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
"@types/node": "^16.18.26",
"@types/react": "^18.2.6",
"@types/react-dom": "^18.2.4",
"jquery": "^3.7.0",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"react-hook-form": "^7.45.1",
Expand Down Expand Up @@ -50,7 +49,6 @@
},
"devDependencies": {
"@tailwindcss/forms": "^0.5.4",
"@types/jquery": "^3.5.16",
"@types/js-cookie": "^3.0.3",
"@typescript-eslint/eslint-plugin": "^5.59.8",
"eslint": "^8.42.0",
Expand Down
10 changes: 5 additions & 5 deletions frontend2/src/components/CurrentUserProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import {
type AuthState,
CurrentUserContext,
} from "../contexts/CurrentUserContext";
import * as Api from "../utils/api";
import { removeApiTokens, doApiTokensExist } from "../utils/cookies";
import { removeApiTokens } from "../utils/cookies";
import { loginCheck } from "../utils/api/auth";
import { getUserUserProfile } from "../utils/api/user";

export const CurrentUserProvider: React.FC<{ children: React.ReactNode }> = ({
children,
Expand All @@ -32,13 +33,12 @@ export const CurrentUserProvider: React.FC<{ children: React.ReactNode }> = ({

useEffect(() => {
const checkLoggedIn = async (): Promise<void> => {
// check if cookies exist before attempting to load user
if (!doApiTokensExist()) {
if (!(await loginCheck())) {
logout();
return;
}
try {
const user = await Api.getUserUserProfile();
const user = await getUserUserProfile();
login(user);
} catch (error) {
logout();
Expand Down
2 changes: 1 addition & 1 deletion frontend2/src/contexts/CurrentUserContext.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { createContext, useContext } from "react";
import { type UserPrivate } from "../utils/types/model/models";
import { type UserPrivate } from "../utils/types";

export enum AuthStateEnum {
LOADING = "loading",
Expand Down
Loading