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.

Sign up for GitHub

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 feature of DLC handling #51

Open
wants to merge 3 commits into
base: Boderator3.0
Choose a base branch
from
Open

New feature of DLC handling #51

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
46d96c5
New feature of DLC handling
Smieszkokoleszko May 22, 2021
4069d5c
Merge branch 'Boderator3.0' into user-dlc
3Mydlo3 Sep 9, 2021
f26186e
Move new files
3Mydlo3 Sep 9, 2021
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: 2 additions & 1 deletion ArmaForces.Boderator/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@
Boderator/*
BoderatorTest/*
BoderatorWeb/*
BoderatorWebTest/*
BoderatorWebTest/*
log*.txt
162 changes: 162 additions & 0 deletions ArmaForces.Boderator/ArmaForces.Boderator.BotService/Controllers/DLCController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
using System;
using System.Linq;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Newtonsoft.Json.Linq;
using System.Collections.Generic;
using System.IO;
using ArmaForces.Boderator.BotService.DataClasses;

namespace ArmaForces.Boderator.BotService.Controllers
{
[Route("users/")]
[ApiController]
[Produces("application/json")]
Copy link
Member

Choose a reason for hiding this comment

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

This is redundant, you can remove this.

Suggested change
[Produces("application/json")]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Redundant with what? Something in rest of program?

Copy link
Member

Choose a reason for hiding this comment

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

You don't need to specify what Content-Type you produce if you just use IActionResult and return the data object. It will be serialized to any Content-Type desired by caller.

public class DLCController : ControllerBase
Copy link
Member

Choose a reason for hiding this comment

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

Is it DLCController with users path? You probably should have created 2 controllers. One for users, one for dlc.

{
private readonly Users _users;

public DLCController(Users users)
{
_users = users;
}

//Adds DLC to user
[HttpPut("{userID}/dlcadd/{dlcName}")]
public void AddDlcToUser(ulong userID, SingleUser.DLC dlcName)
{
if (!(_users.UsersList.Any(x => x.UserID == userID)))
{
Response.StatusCode = 404;
Response.WriteAsync($"User not found");
return;
}

if (_users.UsersList.Where(x => x.UserID == userID).Any(x => x.DLCList.Contains(dlcName)))
{
Response.WriteAsync($"DLC already added");
return;
}

_users.UsersList.Find(x => x.UserID == userID).DLCList.Add(dlcName);

Response.WriteAsync($"DLC added");
return;
}

//List user with given ID. "0" lists all users
[HttpGet("{userID}")]
public void UserOnServer(ulong userID)
Copy link
Member

Choose a reason for hiding this comment

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

No. You should have separate method for no input instead of special handling for 0 value. And method should be named GetUser. Then method for all users would just be named GetUsers or GetAllUsers.

{
JArray usersArray = new JArray();
var usersOnServer = _users.UsersList;

if (userID != 0)
{
usersOnServer = usersOnServer
.FindAll(x => x.UserID == userID);
}

if (usersOnServer.Count == 0)
{
Response.StatusCode = 404;
Response.WriteAsync("User not found");
return;
}

foreach (var user in usersOnServer)
{
var objUser = new JObject();
objUser.Add("UserID", user.UserID);

var dlcArray = new JArray();

foreach (var dlc in user.DLCList)
{
var objDLC = new JObject();
objDLC.Add("DLC", dlc.ToString());
dlcArray.Add(objDLC);
}
objUser.Add("DLC List", dlcArray);

usersArray.Add(objUser);
}
Copy link
Member

Choose a reason for hiding this comment

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

If you need to return users, just return users, don't build the json manually.


Response.WriteAsync($"{usersArray.ToString()}");
}

//Finds users wiht given DLC
[HttpGet("{dlcName}/users")]
public void UsersWithDLC(String dlcName)
{
var usersArray = new JArray();
Copy link
Member

Choose a reason for hiding this comment

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

If you need a list, just create a List<T>.

var usersOnServer = _users.UsersList;

SingleUser.DLC dlcClass;

try
{
dlcClass = (SingleUser.DLC)Enum.Parse(typeof(SingleUser.DLC), dlcName);
}
catch
{
Response.StatusCode = 404;
Response.WriteAsync("DLC not found");
return;
}

foreach (var user in usersOnServer)
{
if (user.DLCList.Contains(dlcClass))
{
var objUser = new JObject();
objUser.Add("UserID", user.UserID);
usersArray.Add(objUser);
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to kill me? Create some response DTO (e.g. UsersWithDLCReponseDto) which will contain the list of users and just return the damn object, don't create JObjects from scratch.

}
}
Copy link
Member

Choose a reason for hiding this comment

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

This whole foreach loop could be made in LINQ.

var usersWithDlc = usersOnServer
    .Where(user => user.DLCList.Contains(dlcClass))
    .ToList();


Response.WriteAsync($"{usersArray.ToString()}");
Copy link
Member

Choose a reason for hiding this comment

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

Return the response object, don't use Response.

}

//Check if user has all DLCs
[HttpGet("DLC")]
public IActionResult UserhasDLC(ulong userID, String dlcs)
{
if (dlcs == null)
{
return (IActionResult)NotFound("No given DLC");
}

List<SingleUser.DLC> dlcList = new List<SingleUser.DLC>();

foreach (string singleDlc in dlcs.Split(','))
{

try
{
var dlcClass = (SingleUser.DLC)Enum.Parse(typeof(SingleUser.DLC), singleDlc);
dlcList.Add(dlcClass);
}
catch
{
return (IActionResult)BadRequest("Wrong DLC name");
}

}

var user = _users.UsersList.SingleOrDefault(x => x.UserID == userID);

if (user is null)
{
return (IActionResult)NotFound("User not found");
}

var dlcFound = !dlcList.Except(user.DLCList).Any();

return dlcFound
? Ok("User has all given DLCs")
: (IActionResult)NotFound("User doesn't have all given DLCs");
}
Copy link
Member

Choose a reason for hiding this comment

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

This is probably useless in API. Instead just make sure API can return user and his DLCs.

}
}

26 changes: 26 additions & 0 deletions ArmaForces.Boderator/ArmaForces.Boderator.BotService/DataClasses/SingleUser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using System.Collections.Generic;

namespace ArmaForces.Boderator.BotService.DataClasses
Copy link
Member

Choose a reason for hiding this comment

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

The DataClasses is awful and stinks with legacy ArmaforcesMissionBot, don't. You are creating a DLC feature so almost everything you do should be in Features/DLC folder/namespace. This one will probably go to Models subfolder.

{
public class SingleUser
Copy link
Member

Choose a reason for hiding this comment

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

Why not just User? This sounds awful.

{
public ulong UserID;
public List<DLC> DLCList = new List<DLC>();
Copy link
Member

Choose a reason for hiding this comment

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

This should be a property too. Don't use fields for publicly available stuff. And make both the property and the list readonly, e.g. IReadOnlyList, IReadOnlyCollection or maybe even IReadOnlySet as it should not have duplicates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about adding new DLC when property is readonly?

Or you meant something like that?

private List<DLC> _dlcList;
public ReadOnlyCollection<DLC> DLCList {get { return _dlcList.AsReadOnly; }}

Copy link
Member

Choose a reason for hiding this comment

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

Yes, almost, something like this:

private List<DLC> _dlcList;
public IReadOnlyCollection<DLC> DLCList => _dlcList.AsReadOnly();

We don't want to allow external modifications to any User or his data. This should be only done in IUserRepository or something like this, right where it's stored. So we need to always return immutable User object. Then to change anything, you must go through IUserRepository which will create new User instance with updated data.


public enum DLC
{
Karts,
Helicopters,
Marksman,
Apex,
Jets,
LawsOfWar,
Tanks,
Contact,
GlobalMobilization,
SOGPrairieFire,
CSLA
}

}
}
10 changes: 10 additions & 0 deletions ArmaForces.Boderator/ArmaForces.Boderator.BotService/DataClasses/Users.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
using System.Collections.Generic;

namespace ArmaForces.Boderator.BotService.DataClasses
{
public class Users
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of this class? It doesn't have an interface. If you want some class which will allow you to retrieve users data then create an interface for it, e.g IUsersRepository and create some useful methods there instead of just returning a list of users.

{
public List<SingleUser> UsersList = new List<SingleUser>();

}
}