Laden...

Thread sicherer Zugriff auf Datenbanken

Erstellt von SteffoD vor 6 Jahren Letzter Beitrag vor 6 Jahren 3.054 Views
S
SteffoD Themenstarter:in
44 Beiträge seit 2018
vor 6 Jahren
Thread sicherer Zugriff auf Datenbanken

Hi an alle,

ich habe eine HTTP Schnittstelle die nach User-Rollen sucht.

Es gibt hierbei einen Spezialfall: Wenn die DB leer ist, dann wird automatisch ein Benutzer mit der Rolle "Admin" erstellt. Diese Prüfung ist vom Design her nicht thread-safe. Deshalb habe ich ein statisches Objekt erstellt das als Lock dient, siehe den unteren Code:

public class UserManagement : Controller
{
	private readonly QlcContext _dbContext;
	private readonly ILogger<UserManagement> _logger;
	private static Object userRoleCheckLock = new Object(); 

	public UserManagement(QlcContext dbContext, ILogger<UserManagement> logger)
	{
		_dbContext = dbContext;
		_logger = logger;
	}

	[HttpGet]
	[Route("GetRole")]
	public IActionResult GetRole(string user)
	{
		if (String.IsNullOrEmpty(user))
		{
			_logger.LogError("Missing user parameter");
			return BadRequest("Missing user parameter");
		}

		QlcUser foundUser;

		lock (userRoleCheckLock)
		{
			foundUser = _dbContext.QlcUser.SingleOrDefault(u => string.Equals(u.Name, user, StringComparison.InvariantCultureIgnoreCase));

			if (foundUser == null)
			{
				// If the user table is empty, then the first user which logs in, is admin else it is an normal user
				bool isAdmin = _dbContext.QlcUser.Count() == 0;
				Role role = isAdmin ? Role.Admin : Role.User;
				foundUser = new QlcUser { Name = user, Role = role };

				Log(Severity.TRACE,
					$"Database user table is empty. Creating first user '{user}' with the role '{foundUser.RoleDisplayName}'",
					user);

				_dbContext.QlcUser.Add(foundUser);
				_dbContext.SaveChanges();
			}
		}

		return Ok(foundUser.Role);
	}
	
	private void Log(Severity severity, string message, string requestor)
	{
		AuditTrail auditTrail = new AuditTrail(
			severity,
			DateTime.UtcNow,
			requestor,
			message);

		_dbContext.AuditTrail.Add(auditTrail);
		_dbContext.SaveChanges();
	}
}

Das Problem ist, dass ich regelmäßg Abstürze bekomme. Die komplette ASP.Net Anwendung stürzt ohne jegliche Exceptions und ohne jegliche Logs ab! Wie ist das möglich? Sollte das nicht funktionieren?!

Danke im Voraus für hilfreiche Tipps!

Viele Grüße,
SteffoD

T
2.219 Beiträge seit 2008
vor 6 Jahren

Hast du mal geschaut, was die Ereignis Anzeige sagt?
Wenn deine ASP .NET Anwendung unerwartet crasht, steht eigentlich immer eine entsprechende Meldung samt Exception drin.

Nachtrag:
Hast du deinen Code auch mal debuggt?
Sieht zwar auf den ersten Blick korrekt aus, aber erst ein Debug Lauf sollte Sicherheit geben.

T-Virus

Developer, Developer, Developer, Developer....

99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.

S
SteffoD Themenstarter:in
44 Beiträge seit 2018
vor 6 Jahren

Hi T-Virus,
danke, für deine Antwort. Ja, ich habe den Code nochmal gedebuggt und er macht was er soll.
Bei der Windows-Ereignis-Anzeige konnte ich nichts finden. Sowohl unter der Rubrik "Fehler" als auch unter "Kritisch", "Warnung" und "Anwendung" nichts.

Ziemlich frustrierend das Ganze. Das beschäftigt mich schon seit Wochen...

Viele Grüße,
Steffo

T
2.219 Beiträge seit 2008
vor 6 Jahren

Klingt aber, als würdest du die entscheidenen Exceptions fangen aber nicht loggen.
Eigentlich müsstest du beim Startup entsprechende Logging Methoden un Exception Filter hinterlegen.
Wenn du nichts hinterlegst, müsstest du eigentlich Meldungen sehen.

Aber Einträge zum Start sollten schon in der Ereignisanzeige stehen oder?

T-Virus

Developer, Developer, Developer, Developer....

99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.

P
1.090 Beiträge seit 2011
vor 6 Jahren

Hi SteffoD,

probier doch mal um die einen Try Catch Block in der ganzen Methode. Vielleicht bekommst du da einen Fehler.

Mal eine andere Sachen, denn ersten Benutzer der sich Anmeldet Admin Rechte zu geben halte ich für sehr Frag würdig. Ihr könnt doch einfach beim erstellen der DB einen Defoult Benutzer anlegen, der Adminrechte hat.

Sollte man mal gelesen haben:

Clean Code Developer
Entwurfsmuster
Anti-Pattern

2.298 Beiträge seit 2010
vor 6 Jahren

Dann wäre mein Vorschlag mal alle unbehandelten Ausnahmen zu fangen und in eine Log-Datei zu schreiben, um erst einmal zu ermitteln wo die Anwendung denn überhaupt Crasht.

Statische Lock-Objekte halte ich gerade im Web für Grenzwertig. Da gibt es mit Sicherheit bessere herangehensweisen.

Haben denn verschiedene Clients die gleiche Controller-Instanz? Ansonsten ist doch hier eine Thread-Synchronisierung überhaupt nicht zielführend da die Clients sowieso unter verschiedenen Kontexten laufen.

Wissen ist nicht alles. Man muss es auch anwenden können.

PS Fritz!Box API - TR-064 Schnittstelle | PS EventLogManager |

S
SteffoD Themenstarter:in
44 Beiträge seit 2018
vor 6 Jahren

Nicht gefangene Exceptions werden geloggt. Siehe den anderen Thread.

Soweit ich weiß, wird für jeden Request eine neue Controller-Instanz erstellt. Daher mein Gedanke, dass der Lock statisch ist. Auch wenn der statische Lock nicht sauber ist, dürfte er eigentlich nicht falsch sein.
Der DBContext ist ebenfalls _kein _Singleton, wie hier zu sehen ist:

public void ConfigureServices(IServiceCollection services)
{
	services.AddDbContext<QlcContext>();
	
	services.AddMvc();
}

Ein try/catch um den Funktions-Rumpf hat auch nicht geholfen:

[HttpGet]
[Route("GetRole")]
public IActionResult GetRole(string user)
{
	if (String.IsNullOrEmpty(user))
	{
		_logger.LogError("Missing user parameter");
		return BadRequest("Missing user parameter");
	}

	try
	{
		QlcUser foundUser;

		lock (userRoleCheckLock)
		{
			foundUser = _dbContext.QlcUser.SingleOrDefault(u => string.Equals(u.Name, user, StringComparison.InvariantCultureIgnoreCase));

			if (foundUser == null)
			{
				// If the user table is empty, then the first user which logs in, is admin else it is an normal user
				bool isAdmin = _dbContext.QlcUser.Count() == 0;
				Role role = isAdmin ? Role.Admin : Role.User;
				foundUser = new QlcUser { Name = user, Role = role };

				Log(Severity.TRACE,
					$"Database user table is empty. Creating first user '{user}' with the role '{foundUser.RoleDisplayName}'",
					user);

				_dbContext.QlcUser.Add(foundUser);
				_dbContext.SaveChanges();
			}
		}

		return Ok(foundUser.Role);
	}
	catch (Exception ex)
	{
		_logger.LogError(ex.ToString());
	}
	return BadRequest();
}

Aber Einträge zum Start sollten schon in der Ereignisanzeige stehen oder?

Ich sehe da nichts. Ist das normal?

Haben denn verschiedene Clients die gleiche Controller-Instanz? Ansonsten ist doch hier eine Thread-Synchronisierung überhaupt nicht zielführend da die Clients sowieso unter verschiedenen Kontexten laufen.

Haben sie nicht, deswegen der statische Lock. Es soll kein gleichzeitiger DB-Look-Up gemacht werden.

16.806 Beiträge seit 2008
vor 6 Jahren

*Eigentlich* machst Du was falsch, wenn Du ein Locking brauchst.
Da solltest eigentlich schon die ersten Warnlampen angehen, dass hier womöglich am Konzept was nicht stimmt.

Warum prüfst Du nicht im Startup der Anwendung nach der DB-Gültigkeit und führst dort einen Add durch?
Wenn Du das jedes Mal in einem Request prüfen musst, dann ist das ein absolut unnötiger Overhead.

Mit Deinem Locking-Versuch untergräbst Du das gesamte Multi-Threading-Verhalten einer Webapplikation - komplett.
Also versteh mich nicht falsch; das ist kein "kleiner Konzeptfehler" - das ist ein richtig grober Schnitzer.
Noch deutlicher kann ich es nicht ausdrücken, ohne einen sehr direkten Begriff zu wählen 😉

Selbst wenn in einer Get-Methode ein Add erfolgt - was völlig haarsträubend ist in meinen Augen - braucht man kein Locking.
Wenn die Datenbank ordentlich modelliert ist, kann jeder User - Eindeutigkeit durch den Namen - nur ein Mal erstellt werden.
Bei einem zweiten Add würde es demnach eine Exception geben, auf die man reagieren kann.
Das Locking ist also aus allen möglichen Blickwinkeln technisch nicht notwendig - hier sogar im Gegenteil: es ist falsch. ⚠

PS: .NET Core allein kann nicht in die Windows Ereignisanzeige loggen.
Hat es nie und wird es nicht. Das gilt nur für die alte Welt im IIS Kontext.
Ist dahingehend natürlich dann auch völlig Banane, da zu schauen.
ASP.NET Core loggt vollständig in den ILogger. 👍

S
SteffoD Themenstarter:in
44 Beiträge seit 2018
vor 6 Jahren

Danke, für deine Antwort.
Ich bin dennoch der Ansicht, dass man um das eigentliche Problem drum herum redet. D. h.: Weshalb stürzt die Anwendung komplett ab, wenn Locking genutzt? Kann das auch in anderen Konstellationen auftauchen?

Meine Funktion fügt nicht nur einen Admin hinzu, sondern auch normale User mit der Rolle "User" sofern sie in der DB nicht existieren. Dieses Verhalten halte ich für sinnvoll, da das eine Intranet-Anwendung ist, in der User automatisch hinzugefügt werden sollen, sofern sie in der DB nicht existieren und nur der erste User wird Admin.

Locking ist außerdem GERADE in Multithreading-Anwendung ein absolut legitimes Mittel, da man das bei synchronen Anwendungen sowieso nicht braucht. 😉
Von daher kann ich deine Einwände nicht richtig nachvollziehen außer dass man natürlich versuchen sollte das Locking zu minimieren, sofern möglich.

16.806 Beiträge seit 2008
vor 6 Jahren

Ich bin dennoch der Ansicht, dass man um das eigentliche Problem drum herum redet.

Das Grundproblem löst sich von allein, wenn Du es ordentlich umsetzt 😉

D. h.: Weshalb stürzt die Anwendung komplett ab, wenn Locking genutzt?

Am Lock allein wird es nicht liegen.

Kann das auch in anderen Konstellationen auftauchen?

Es könnte auch an der Erde-Mond-Konstellation liegen - ohne Exception müssen auch wir raten.
Auch wir haben keine Glaskugel 😉

Meine Funktion fügt nicht nur einen Admin hinzu, sondern auch normale User mit der Rolle "User" sofern sie in der DB nicht existieren.

Auch das ist nicht die Aufgabe einer Action.
Das finde ich an der Stelle sogar kurios:

Wieso werden - wenn Du schon offensichtlich in einer Intranet-Umgebung mit Active Directory bist - die Gruppen nicht über die Rollen eines AD-Users gemacht?
Wieso gibt es die User nun doppelt?
Wie bekommst Du mit, wenn sich ein User im AD ändert? Wie synchronisierst Du das?
Das zu duplizieren ist der erste Schritt in ein ungültiges Datengebilde 😃

Dieses Verhalten halte ich für sinnvoll, da das eine Intranet-Anwendung ist, in der User automatisch hinzugefügt werden sollen, sofern sie in der DB nicht existieren und nur der erste User wird Admin.

Trotzdem braucht man kein Locking.

Locking ist außerdem GERADE in Multithreading-Anwendung ein absolut legitimes Mittel, da man das bei synchronen Anwendungen sowieso nicht braucht. 😉

Locking ist das letzte Mittel in einer Multi-Thread-Anwendung.
Hier ist es technisch nicht notwendig. Es ist technisch sogar falsch.
Du untergräbst damit das GESAMTE Multi-Threading-Verhalten aller lock-beteiligten Actions.

Und wie gesagt: für die Eindeutigkeit hier ist das Datenbank-Schema verantwortlich.
Wieso nutzt Du es nicht? Allein diese Mini-Änderung macht Dein Locking unnötig.

Von daher kann ich deine Einwände nicht richtig nachvollziehen

Weil Du mir nicht glaubst, oder Du es technisch nicht nachvollziehen kannst?
Was hab ich davon Dir Quatsch zu erzählen? 😃

S
SteffoD Themenstarter:in
44 Beiträge seit 2018
vor 6 Jahren

Ich könnte die Task fein sauber in drei Tasks unterteilen:

  • bool UserExists(string user): Existiert ein User in der DB?
  • void AddUser(string user): Falls nein, füge den User hinzu
  • Role GetRole(string user): Gibt lediglich die Rolle eines Users zurück

Das sind drei Requests die zusammengenommen mehr Performance kosten würden.
Weiter kommt noch hinzu, dass GetRole() eine Funktion ist, die nur bei Programmstart des Client ca. 3-4 mal von 3-4 Tasks gleichzeitig aufgerufen wird. Danach wird das Ergebnis von der jeweiligen Klasse gecacht. Ist also ein seltener Aufruf, wo ich mir über Performance keine großen Gedanken machen muss.
Was ich eher machen würde: Den Request in meinem Client nur einmal zu machen und das Ergebnis global zu cachen.

Wie würdest du das machen?

S
SteffoD Themenstarter:in
44 Beiträge seit 2018
vor 6 Jahren

Und wie gesagt: für die Eindeutigkeit hier ist das Datenbank-Schema verantwortlich.
Wieso nutzt Du es nicht? Allein diese Mini-Änderung macht Dein Locking unnötig.

Was meinst du mit Eindeutigkeit? Die Usernamen sind bei mir eindeutig und wenn ich versuche denselben User nochmals hinzuzufügen, wird eine Exception geworfen.
Da bei mir der Client beim Start 3-4 Requests gleichzeitig startet und nach der Rolle fragt, kam es bei mir ohne Locking schon zu Exceptions, da ein gerade hinzugefügter User nochmals hinzugefügt wurde.

Weil Du mir nicht glaubst, oder Du es technisch nicht nachvollziehen kannst?
Was hab ich davon Dir Quatsch zu erzählen? 😃

Ich meinte, dass Locking ausschließlich bei Multithreading-Anwendungen Sinn macht. Du hast angedeutet, dass man gerade dort kein Locking verwenden sollte.

16.806 Beiträge seit 2008
vor 6 Jahren

Da bei mir der Client beim Start 3-4 Requests gleichzeitig startet und nach der Rolle fragt, kam es bei mir ohne Locking schon zu Exceptions, da ein gerade hinzugefügter User nochmals hinzugefügt wurde.

WAS für eine Exception? Ich kann leider immer noch nicht hellsehen 😉
Eine Exception, dass der Benutzer bereits existiert?
BINGO. Genau das willst Du.

Wenn Du diese Exception bekommst, dann lade doch an der Stelle den Benutzer nochmal erneut und gebe den dann zurück: fertig.

Da brauchst Du kein Locking.
Sowas nennt sich transient-fault-handling.

Wenn gegenüber einem Netzwerk-Element (Datenbank, API...) einen Fehler wirft, versucht man es X mal nochmals.
Es gibt dazu auch ein offizielles .NET Projekt, mit dem man sowas automatisieren könnte:
https://github.com/App-vNext/Polly

Aber im Endeffekt machst Du nichts anderes als auf die Exception zu hören die sagt, ob der Nutzer schon in der DB existiert.
Wenn diese kommt, dann lädst Du einfach nochmals das Benutzer-Objekt.
Es muss ja dann existieren.

[HttpGet]
[Route("GetRole")]
public IActionResult GetRole(string user) {
 if (String.IsNullOrEmpty(user)) {
  _logger.LogError("Missing user parameter");
  return BadRequest("Missing user parameter");
 }

 var maxRetry = 3;
 var count = 0;
 do {
   count++;

  foundUser = _dbContext.QlcUser.SingleOrDefault(u => string.Equals(u.Name, user, StringComparison.InvariantCultureIgnoreCase));

  if (foundUser == null) {
   // If the user table is empty, then the first user which logs in, is admin else it is an normal user
   bool isAdmin = _dbContext.QlcUser.Count() == 0;
   Role role = isAdmin ? Role.Admin : Role.User;
   foundUser = new QlcUser {
    Name = user, Role = role
   };

   Log(Severity.TRACE,
    $ "Database user table is empty. Creating first user '{user}' with the role '{foundUser.RoleDisplayName}'",
    user);

   try {
    _dbContext.QlcUser.Add(foundUser);
    _dbContext.SaveChanges();
   } catch (UserAlreadyExistsException e) {
    // WHOOOOPSI!
    // user existiert wohl doch schon?!
    // nochmals versuchen -> polly

   }
  }
  while (count <= maxRetry);
  // exception falls es 3 mal schief ging

 }
 return Ok(foundUser.Role);

 return BadRequest();
}

Das packste dann noch sauber in die Business Logik und gut ist.
[Artikel] Drei-Schichten-Architektur
Eine Action ist keine Logik-Schicht sondern gehört zur UI-Schicht.

Weil Du mir nicht glaubst, oder Du es technisch nicht nachvollziehen kannst?
Was hab ich davon Dir Quatsch zu erzählen? 😃
Ich meinte, dass Locking ausschließlich bei Multithreading-Anwendungen Sinn macht. Du hast angedeutet, dass man gerade dort kein Locking verwenden sollte.

Man sollte es wenn immer möglich vermeiden.
-> Hier kann man es sehr sehr einfach vermeiden.

Und PS: bitte verwende asynchrone Methoden wo immer Du nur kannst.
In ASP.NET Core werden bald nur noch asynchrone Actions möglich sein (so wie es bei mobilen Anwendungen auch heute schon ist).

Und PPS:
Schau Dir REST an. GetRole ist keine REST-konformes URL-Segment.

S
SteffoD Themenstarter:in
44 Beiträge seit 2018
vor 6 Jahren

Da bei mir der Client beim Start 3-4 Requests gleichzeitig startet und nach der Rolle fragt, kam es bei mir ohne Locking schon zu Exceptions, da ein gerade hinzugefügter User nochmals hinzugefügt wurde.
WAS für eine Exception? Ich kann leider immer noch nicht hellsehen 😉
Eine Exception, dass der Benutzer bereits existiert?
BINGO. Genau das willst Du.

Wenn Du diese Exception bekommst, dann lade doch an der Stelle den Benutzer nochmal erneut und gebe den dann zurück: fertig.

Das werde ich mal versuchen, danke.

Und PS: bitte verwende asynchrone Methoden wo immer Du nur kannst.
In ASP.NET Core werden bald nur noch asynchrone Actions möglich sein (so wie es bei mobilen Anwendungen auch heute schon ist).

Ich hatte vorher alles mit Async-Methoden. Der Witz war aber, dass davor immer ein await war. Also z. B. halte ich await SingleOrDefaultAsync() für witzlos, da die Methode durch await foo() synchron ausgeführt wird. Durch den Overhead kann die Applikation dann langsamer laufen. Siehe diesen Issue. In dem Issue wird auch gezeigt, wie man es richtig macht, nur ergibt sich diese Möglichkeit bei mir nicht, da ich in der nächsten Zeile schon wieder das Ergebnis verarbeiten muss. Es nützt also wenig, wenn ich eine Task losschicke und in der nächsten Zeile auf die Task warte. Was dagegen hilft ist eine Task loszuschicken und währenddessen etwas anderes zu machen und irgendwann ein await aufrufen. Oder sehe ich das falsch?

16.806 Beiträge seit 2008
vor 6 Jahren

Der Witz war aber, dass davor immer ein await war.

Das ist korrekt.

Also z. B. halte ich await SingleOrDefaultAsync() für witzlos, da die Methode durch await foo() synchron ausgeführt wird.

Nein, sie ist dadurch asynchron - aber nicht parallel.
Das sind zwei paar Stiefel.

Schau Dir bitte an, was async/await ist.
Der Link, den Du da zeigst - das sieht man an den Kommentaren - zeigt zudem, dass auch die Leute async/await nicht wirklich verstanden haben und auch das Messverhalten nicht korrekt anwenden.
Siehe allein die Tatsache, dass .Result kein legitimer Pattern ist und zu Messfehlern führt.

Durch den Overhead kann die Applikation dann langsamer laufen.

Nein.

Ja, die StateMachine hinter async/await kostet Zeit - aber der positive Effekt im Gesamten ist höher als der Overhead - vor allem bei Webapplikationen.
Und: Du wirst ohnehin bald dazu gezwungen damit zu arbeiten - und das ist gut so.

Ohne jetzt zu direkt zu werden: glaubst Du wirklich, dass Du APIs entwickelst, auf die es ankommt, dass sie 2ms schneller antworten? 😉
.. ich hab da so meine Zweifel 😃

Das gesamte ASP.NET Konstrukt basiert auf async/await.
Rational: Glaubst Du wirklich, dass diese sehr sehr schlauen Menschen dahinter async/await verwenden, nur damit es langsamer ist?

Real: Du beschwerst Dich über den Overhead von async/await aber dann verwendest Entity Framework Core, das alles andere als bekannt für seine Performance ist.
Da hakt ein wenig die Argumentationsbasis 😉
Du hast Potentiale im Code was die Verbesserung betrifft; aber um Performane kümmert man sich erst, wenn man muss.
=> premature optimization is the root of all evil

Deswegen auch: Finger weg von Caching, wenn man nicht dazu gezwungen wird.

Oder sehe ich das falsch?

Ja.

Du beschreibst hier mit dem Wunsch an Task Parallelität ("Task loszuschicken und währenddessen etwas anderes zu machen"): async/await hat aber mit Parallelität nichts zutun.
Insgesamt zeigt mir Deine gesamte Argumentation hier: Du hast async/await nicht verstanden.

S
SteffoD Themenstarter:in
44 Beiträge seit 2018
vor 6 Jahren

Gut asynchron bedeutet, dass man die Kontrolle wieder dem Aufrufer zurückgibt solange die Task am Arbeiten ist. Das ist bei der GUI z. B. wichtig.
Aber wie ist das bei Kestrel? Wird da nicht sowieso pro Request eine Task erstellt?

16.806 Beiträge seit 2008
vor 6 Jahren

Rumraten bringt nichts, SteffoD.
Bitte schau Dir an, was async/await ist und warum alle Elemente async/await implementieren müssen, damit es was was bringt.

https://docs.microsoft.com/de-de/dotnet/standard/async-in-depth

S
SteffoD Themenstarter:in
44 Beiträge seit 2018
vor 6 Jahren

Du beschreibst hier mit dem Wunsch an Task Parallelität ("Task loszuschicken und währenddessen etwas anderes zu machen"): async/await hat aber mit Parallelität nichts zutun.

Ohne dich jetzt auf die Palme bringen zu wollen: Aber async/await kann asynchron innerhalb eines Threads bedeuten oder auch dass ein neuer Thread kreiert wird und damit ist es dann parallel.
Ich gebe zu, dass ich da am Anfang ungenau war, aber zu sagen, dass async/await nichts mit Parallelität zu tun hat, ist auch nicht richtig.

16.806 Beiträge seit 2008
vor 6 Jahren

Gut, an der Stelle merke ich, dass ich Dich evtl. einfach Deine Fehler machen lassen muss...
Dahingehend wünsche ich Dir viel Erfolg dabei.
Die grundlegenden Informationen das Problem zu lösen, hast Du ja. Den Rest bekommst dann sicher auch hin 😉

S
SteffoD Themenstarter:in
44 Beiträge seit 2018
vor 6 Jahren

Optional können Tasks explizit zur Ausführung auf einem separaten Thread über die Task.Run-API angefordert werden.

Dennoch danke für deine Hilfe. 😃

B
72 Beiträge seit 2009
vor 6 Jahren

Und PPS:
Schau Dir REST an. GetRole ist keine REST-konformes URL-Segment.

Hallo Abt,

was genau meinst du hiermit? Was ist da falsch?

2.207 Beiträge seit 2011
vor 6 Jahren

Hallo bbb,

geht um die URL. Schau dir mal REST an. Die URL hat normalerweise kein "GET" mehr im Namen. Das ist mehr SOAP-Stil. 😉 Das GET legt ja das HTTP-Verb schon fest. Verb, Router und Content-Type sind die Keys bei REST.

Gruss

Coffeebean

16.806 Beiträge seit 2008
vor 6 Jahren

Was ist da falsch?

Es ist nicht falsch, aber es ist auch nicht REST-konform 😉

Korrekt im Sinne von REST hier wäre (aus dem Kontext): /users/{user}/roles

bzw im Ganzen:

[HttpGet("users/{userId}/roles")]
public Task<IActionResult> GetUserRolesAsync(string userId)
{
 ...
}

bzw ab ASP.NET Core 2.1

[HttpGet("users/{userId}/roles")]
public Task<ActionResult<UserRolesApiModel>> GetUserRolesAsync(string userId)
{
 ...
}

Aber in Anbetracht, wie SteffoD offensichtlich entwickelt/entwickeln möchte, ist das evtl. sowieso nur nen Tropfen auf den heißen Stein - zugegeben.

B
72 Beiträge seit 2009
vor 6 Jahren

Super, vielen Dank fürs Klarstellen!