Skip to content

Commit 60e3532

Browse files
YuvalYuval
authored andcommitted
refactor: address architectural review findings across stack
- db: unify event SP aliases (EventType_Name, Vote_Count) with additive backwards-compatible duplicates for zero-downtime rollout - backend: self-only guard on GET /users/{id}; unify AuthController error shape via typed exceptions; remove .Result after Task.WhenAll in GeoController; rename WebApplication1.Options namespace to GroundShareAPI.Options - frontend: align EventVoteResult with server aliases; replace swallowed .catch(() => {}) with structured console.warn logging in AddressSearchMain, NearbyReportsScreen, EventCard
1 parent 91b6a41 commit 60e3532

14 files changed

Lines changed: 106 additions & 100 deletions

File tree

01-Database/GroundShareDB.sql

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -485,8 +485,10 @@ AS
485485
BEGIN
486486
SET NOCOUNT ON;
487487
SELECT e.Event_ID, e.Description, e.EventStatus, e.Start_Date, e.End_Date, e.Picture, e.Created_At,
488+
et.Name AS EventType_Name,
488489
et.Name AS EventType,
489490
u.Full_Name AS Author,
491+
ISNULL((SELECT SUM(Vote) FROM event_vote WHERE Event_ID = e.Event_ID), 0) AS Vote_Count,
490492
ISNULL((SELECT SUM(Vote) FROM event_vote WHERE Event_ID = e.Event_ID), 0) AS VoteCount
491493
FROM event e
492494
INNER JOIN event_type et ON e.EventType_ID = et.EventType_ID
@@ -572,8 +574,10 @@ BEGIN
572574
END
573575
END
574576

575-
-- Return new total
576-
SELECT ISNULL(SUM(Vote), 0) AS VoteCount FROM event_vote WHERE Event_ID = @Event_ID;
577+
-- Return new total (both aliases during additive migration)
578+
SELECT ISNULL(SUM(Vote), 0) AS Vote_Count,
579+
ISNULL(SUM(Vote), 0) AS VoteCount
580+
FROM event_vote WHERE Event_ID = @Event_ID;
577581
END
578582
GO
579583

02-Server/Controllers/AuthController.cs

Lines changed: 55 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
using System.Text.RegularExpressions;
2222
using Google.Apis.Auth;
2323
using Microsoft.Extensions.Options;
24-
using WebApplication1.Options;
24+
using GroundShareAPI.Options;
2525

2626
namespace GroundShareAPI.Controllers
2727
{
@@ -75,39 +75,42 @@ public async Task<IActionResult> Register([FromBody] RegisterRequest request)
7575
var streetName = Sanitize(request.Street_Name);
7676
var houseNumber = Sanitize(request.House_Number);
7777

78-
// --- Email validation ---
78+
// Validation errors flow through ExceptionHandlingMiddleware so the
79+
// client sees one ApiResponse<object> shape for every failure.
80+
var fieldErrors = new Dictionary<string, string>();
81+
7982
if (string.IsNullOrEmpty(email))
80-
return BadRequest(new { field = "email", message = "כתובת מייל היא שדה חובה" });
81-
if (email.Length > 254)
82-
return BadRequest(new { field = "email", message = "כתובת מייל ארוכה מדי" });
83-
if (!EmailRegex.IsMatch(email))
84-
return BadRequest(new { field = "email", message = "כתובת מייל לא תקינה" });
83+
fieldErrors["email"] = "כתובת מייל היא שדה חובה";
84+
else if (email.Length > 254)
85+
fieldErrors["email"] = "כתובת מייל ארוכה מדי";
86+
else if (!EmailRegex.IsMatch(email))
87+
fieldErrors["email"] = "כתובת מייל לא תקינה";
8588

86-
// --- Password validation ---
8789
if (string.IsNullOrEmpty(password))
88-
return BadRequest(new { field = "password", message = "סיסמה היא שדה חובה" });
89-
if (password.Length < 8)
90-
return BadRequest(new { field = "password", message = "הסיסמה חייבת להכיל לפחות 8 תווים" });
91-
if (password.Length > 128)
92-
return BadRequest(new { field = "password", message = "הסיסמה ארוכה מדי (מקסימום 128 תווים)" });
93-
if (!password.Any(char.IsUpper))
94-
return BadRequest(new { field = "password", message = "הסיסמה חייבת לכלול לפחות אות גדולה באנגלית" });
95-
if (!password.Any(char.IsLower))
96-
return BadRequest(new { field = "password", message = "הסיסמה חייבת לכלול לפחות אות קטנה באנגלית" });
97-
if (!password.Any(char.IsDigit))
98-
return BadRequest(new { field = "password", message = "הסיסמה חייבת לכלול לפחות ספרה אחת" });
99-
100-
// --- Full name validation ---
90+
fieldErrors["password"] = "סיסמה היא שדה חובה";
91+
else if (password.Length < 8)
92+
fieldErrors["password"] = "הסיסמה חייבת להכיל לפחות 8 תווים";
93+
else if (password.Length > 128)
94+
fieldErrors["password"] = "הסיסמה ארוכה מדי (מקסימום 128 תווים)";
95+
else if (!password.Any(char.IsUpper))
96+
fieldErrors["password"] = "הסיסמה חייבת לכלול לפחות אות גדולה באנגלית";
97+
else if (!password.Any(char.IsLower))
98+
fieldErrors["password"] = "הסיסמה חייבת לכלול לפחות אות קטנה באנגלית";
99+
else if (!password.Any(char.IsDigit))
100+
fieldErrors["password"] = "הסיסמה חייבת לכלול לפחות ספרה אחת";
101+
101102
if (string.IsNullOrEmpty(fullName))
102-
return BadRequest(new { field = "fullName", message = "שם מלא הוא שדה חובה" });
103-
if (fullName.Length < 2)
104-
return BadRequest(new { field = "fullName", message = "שם חייב להכיל לפחות 2 תווים" });
105-
if (fullName.Length > 50)
106-
return BadRequest(new { field = "fullName", message = "שם ארוך מדי (מקסימום 50 תווים)" });
103+
fieldErrors["fullName"] = "שם מלא הוא שדה חובה";
104+
else if (fullName.Length < 2)
105+
fieldErrors["fullName"] = "שם חייב להכיל לפחות 2 תווים";
106+
else if (fullName.Length > 50)
107+
fieldErrors["fullName"] = "שם ארוך מדי (מקסימום 50 תווים)";
107108

108-
// --- Phone validation (optional) ---
109109
if (!string.IsNullOrEmpty(phone) && !PhoneRegex.IsMatch(phone))
110-
return BadRequest(new { field = "phone", message = "מספר טלפון לא תקין" });
110+
fieldErrors["phone"] = "מספר טלפון לא תקין";
111+
112+
if (fieldErrors.Count > 0)
113+
throw new Exceptions.ValidationException("נתוני הרשמה אינם תקינים", fieldErrors);
111114

112115
try
113116
{
@@ -121,7 +124,7 @@ public async Task<IActionResult> Register([FromBody] RegisterRequest request)
121124
string.IsNullOrEmpty(houseNumber) ? null : houseNumber);
122125

123126
if (userRow == null)
124-
return StatusCode(500, new { message = "שגיאה בהרשמה" });
127+
throw new InvalidOperationException("User row was null after successful register.");
125128

126129
int userId = Convert.ToInt32(userRow["User_ID"]);
127130
string accessToken = GenerateAccessToken(userId, email);
@@ -151,7 +154,7 @@ public async Task<IActionResult> Register([FromBody] RegisterRequest request)
151154
public async Task<IActionResult> Login([FromBody] LoginRequest request)
152155
{
153156
if (string.IsNullOrWhiteSpace(request.Email) || string.IsNullOrWhiteSpace(request.Password))
154-
return BadRequest(new { message = "Email and password are required." });
157+
throw new Exceptions.ValidationException("Email and password are required.");
155158

156159
var userRow = await _usersDal.GetUserByEmailRawAsync(request.Email);
157160

@@ -174,7 +177,7 @@ public async Task<IActionResult> Login([FromBody] LoginRequest request)
174177
{
175178
await _audit.LogAsync(null, "LOGIN_FAILURE", ClientIp, ClientUserAgent,
176179
JsonSerializer.Serialize(new { email = request.Email }));
177-
return Unauthorized(new { message = "אימות נכשל" });
180+
throw new UnauthorizedException("אימות נכשל");
178181
}
179182

180183
int userId = Convert.ToInt32(userRow["User_ID"]);
@@ -202,7 +205,7 @@ await _audit.LogAsync(null, "LOGIN_FAILURE", ClientIp, ClientUserAgent,
202205
public async Task<IActionResult> GoogleSignIn([FromBody] GoogleSignInRequest request)
203206
{
204207
if (string.IsNullOrWhiteSpace(request?.IdToken))
205-
return BadRequest(new { message = "Missing Google credential." });
208+
throw new Exceptions.ValidationException("Missing Google credential.");
206209

207210
// -----------------------------------------------------------------
208211
// Verify the ID token using Google's official library.
@@ -227,65 +230,58 @@ public async Task<IActionResult> GoogleSignIn([FromBody] GoogleSignInRequest req
227230
});
228231

229232
if (!payload.EmailVerified)
230-
return Unauthorized(new { message = "Google email is not verified." });
233+
throw new UnauthorizedException("Google email is not verified.");
231234

232235
email = payload.Email ?? "";
233236
fullName = payload.Name ?? payload.GivenName ?? email.Split('@')[0];
234237

235238
if (string.IsNullOrWhiteSpace(email))
236-
return Unauthorized(new { message = "Google token missing email." });
239+
throw new UnauthorizedException("Google token missing email.");
237240
}
238241
catch (InvalidJwtException)
239242
{
240-
return Unauthorized(new { message = "Invalid Google token." });
243+
throw new UnauthorizedException("Invalid Google token.");
241244
}
242245

243246
email = Sanitize(email).ToLowerInvariant();
244247
fullName = Sanitize(fullName);
245248
if (fullName.Length > 50) fullName = fullName.Substring(0, 50);
246249
if (string.IsNullOrWhiteSpace(fullName)) fullName = email.Split('@')[0];
247250

248-
try
249-
{
250-
var userRow = await _socialAuthDal.UpsertGoogleUserAsync(email, fullName);
251-
if (userRow == null)
252-
return StatusCode(500, new { message = "Failed to create or load user." });
251+
var userRow = await _socialAuthDal.UpsertGoogleUserAsync(email, fullName);
252+
if (userRow == null)
253+
throw new InvalidOperationException("Failed to create or load Google user.");
253254

254-
int userId = Convert.ToInt32(userRow["User_ID"]);
255-
string accessToken = GenerateAccessToken(userId, email);
256-
string refreshToken = GenerateRefreshToken();
255+
int userId = Convert.ToInt32(userRow["User_ID"]);
256+
string accessToken = GenerateAccessToken(userId, email);
257+
string refreshToken = GenerateRefreshToken();
257258

258-
await _refreshTokenDal.SaveTokenAsync(userId, refreshToken, DateTime.UtcNow.AddDays(7));
259-
await _audit.LogAsync(userId, "GOOGLE_SIGNIN", ClientIp, ClientUserAgent);
259+
await _refreshTokenDal.SaveTokenAsync(userId, refreshToken, DateTime.UtcNow.AddDays(7));
260+
await _audit.LogAsync(userId, "GOOGLE_SIGNIN", ClientIp, ClientUserAgent);
260261

261-
return Ok(new
262-
{
263-
accessToken,
264-
refreshToken,
265-
user = userRow
266-
});
267-
}
268-
catch (Exception)
262+
return Ok(new
269263
{
270-
return StatusCode(500, new { message = "An error occurred during Google sign-in." });
271-
}
264+
accessToken,
265+
refreshToken,
266+
user = userRow
267+
});
272268
}
273269

274270
[HttpPost("refresh")]
275271
[EnableRateLimiting("auth-refresh")]
276272
public async Task<IActionResult> Refresh([FromBody] RefreshRequest request)
277273
{
278274
if (string.IsNullOrWhiteSpace(request.RefreshToken))
279-
return BadRequest(new { message = "Refresh token is required." });
275+
throw new Exceptions.ValidationException("Refresh token is required.");
280276

281277
var tokenRow = await _refreshTokenDal.GetTokenAsync(request.RefreshToken);
282278

283279
if (tokenRow == null)
284-
return Unauthorized(new { message = "Invalid refresh token." });
280+
throw new UnauthorizedException("Invalid refresh token.");
285281

286282
var expiresAt = Convert.ToDateTime(tokenRow["Expires_At"]);
287283
if (expiresAt < DateTime.UtcNow)
288-
return Unauthorized(new { message = "Refresh token expired." });
284+
throw new UnauthorizedException("Refresh token expired.");
289285

290286
// Revoke old token
291287
await _refreshTokenDal.RevokeTokenAsync(request.RefreshToken);
@@ -295,7 +291,7 @@ public async Task<IActionResult> Refresh([FromBody] RefreshRequest request)
295291
// Get user data
296292
var userRow = await _usersDal.GetUserByIdAsync(userId);
297293
if (userRow == null)
298-
return Unauthorized(new { message = "User not found." });
294+
throw new UnauthorizedException("User not found.");
299295

300296
string email = userRow["Email"]?.ToString() ?? "";
301297
string newAccessToken = GenerateAccessToken(userId, email);

02-Server/Controllers/EventsController.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public async Task<IActionResult> Vote(int eventId, [FromBody] VoteRequest reques
5959
{
6060
int userId = GetUserId();
6161
int newCount = await _eventService.VoteOnEventAsync(userId, eventId, request.Vote);
62-
return Ok(new { VoteCount = newCount });
62+
return Ok(new { Vote_Count = newCount, VoteCount = newCount });
6363
}
6464

6565
[HttpGet("{eventId}/comments")]

02-Server/Controllers/GeoController.cs

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -200,16 +200,17 @@ public async Task<IActionResult> GetPlanningStatus([FromQuery] double lat, [From
200200

201201
await Task.WhenAll(tasks.Values);
202202

203-
// Parse results
204-
var constructionSites = ParseFeatures(tasks["499"].Result);
205-
var permits = ParseFeatures(tasks["772"].Result);
206-
var nightWorkConstruction = ParseFeatures(tasks["479"].Result);
207-
var nightWorkPublic = ParseFeatures(tasks["858"].Result);
208-
var roadWorkPoly = ParseFeatures(tasks["852"].Result);
209-
var roadWorkPoint = ParseFeatures(tasks["853"].Result);
210-
var landUseMain = ParseFeatures(tasks["514"].Result);
211-
var landUseDetailed = ParseFeatures(tasks["837"].Result);
212-
var cityPlans = ParseFeatures(tasks["528"].Result);
203+
// Tasks are completed here — await each to surface results without
204+
// resorting to .Result (which re-introduces sync blocking).
205+
var constructionSites = ParseFeatures(await tasks["499"]);
206+
var permits = ParseFeatures(await tasks["772"]);
207+
var nightWorkConstruction = ParseFeatures(await tasks["479"]);
208+
var nightWorkPublic = ParseFeatures(await tasks["858"]);
209+
var roadWorkPoly = ParseFeatures(await tasks["852"]);
210+
var roadWorkPoint = ParseFeatures(await tasks["853"]);
211+
var landUseMain = ParseFeatures(await tasks["514"]);
212+
var landUseDetailed = ParseFeatures(await tasks["837"]);
213+
var cityPlans = ParseFeatures(await tasks["528"]);
213214

214215
// Combine road work
215216
var roadWork = new List<Dictionary<string, object?>>();
@@ -351,11 +352,11 @@ public async Task<IActionResult> GetNearbyDisruptions(
351352

352353
await Task.WhenAll(tasks.Values);
353354

354-
var constructionSites = ParseFeaturesWithGeometry(tasks["499"].Result, "construction");
355-
var nightWorkConst = ParseFeaturesWithGeometry(tasks["479"].Result, "nightWork");
356-
var nightWorkPublic = ParseFeaturesWithGeometry(tasks["858"].Result, "nightWork");
357-
var roadWorkPoly = ParseFeaturesWithGeometry(tasks["852"].Result, "roadWork");
358-
var roadWorkPoint = ParseFeaturesWithGeometry(tasks["853"].Result, "roadWork");
355+
var constructionSites = ParseFeaturesWithGeometry(await tasks["499"], "construction");
356+
var nightWorkConst = ParseFeaturesWithGeometry(await tasks["479"], "nightWork");
357+
var nightWorkPublic = ParseFeaturesWithGeometry(await tasks["858"], "nightWork");
358+
var roadWorkPoly = ParseFeaturesWithGeometry(await tasks["852"], "roadWork");
359+
var roadWorkPoint = ParseFeaturesWithGeometry(await tasks["853"], "roadWork");
359360

360361
var all = new List<object>();
361362
all.AddRange(constructionSites);

02-Server/Controllers/UsersController.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ public async Task<IActionResult> GetMyProfile()
4545
[HttpGet("{id}")]
4646
public async Task<IActionResult> GetUserById(int id)
4747
{
48+
int callerId = GetUserId();
49+
if (callerId != id)
50+
return Forbid();
51+
4852
var user = await _usersDal.GetUserByIdAsync(id);
4953

5054
if (user == null)

02-Server/Options/GoogleApiOptions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
using System.ComponentModel.DataAnnotations;
22

3-
namespace WebApplication1.Options;
3+
namespace GroundShareAPI.Options;
44

55
/// <summary>
66
/// Strongly-typed view of the "Google" configuration section.

02-Server/Options/JwtOptions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
using System.ComponentModel.DataAnnotations;
22

3-
namespace WebApplication1.Options;
3+
namespace GroundShareAPI.Options;
44

55
/// <summary>
66
/// Strongly-typed view of the "Jwt" configuration section.

02-Server/Options/StorageOptions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
using System.ComponentModel.DataAnnotations;
22

3-
namespace WebApplication1.Options;
3+
namespace GroundShareAPI.Options;
44

55
/// <summary>
66
/// Strongly-typed view of the "Storage" configuration section.

02-Server/Program.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
using System.Net;
66
using System.Text;
77
using System.Threading.RateLimiting;
8-
using WebApplication1.Options;
8+
using GroundShareAPI.Options;
99
using GroundShareAPI.Middleware;
1010
using GroundShareAPI.DAL;
1111
using GroundShareAPI.DAL.Interfaces;

02-Server/Services/AzureBlobStorageService.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
using Azure.Storage.Blobs.Models;
2626
using Azure.Storage.Sas;
2727
using Microsoft.Extensions.Options;
28-
using WebApplication1.Options;
28+
using GroundShareAPI.Options;
2929

3030
namespace GroundShareAPI.Services
3131
{

0 commit comments

Comments
 (0)