OK-751 käyttöoikeuskäsittelyn tehostamista #84
Conversation
…a, tyylien päivitystä, kälikomponentteja, mui 6 mirgaatiota
…ielen hakuun cachetusta kälissä
…vitetään sessio tarvittaessa
| * Hakee lähetyksen viestin lukumäärän | ||
| * | ||
| * @param lahetysTunniste lähetyksen tunniste | ||
| * @return lähetyksen viestien määrä |
There was a problem hiding this comment.
Tämä kommentti lienee copypastettu
| @@ -58,8 +58,10 @@ class LahetysResource { | |||
| @RequestParam(name = HAKU_ALKAEN_PARAM_NAME, required = false) hakuAlkaen: Optional[String], | |||
| @RequestParam(name = HAKU_PAATTYEN_PARAM_NAME, required = false) hakuPaattyen: Optional[String], | |||
| request: HttpServletRequest): ResponseEntity[PalautaLahetyksetResponse] = | |||
| val securityOperaatiot = new SecurityOperaatiot | |||
| LOG.info("Haetaan lähetyksiä") | |||
There was a problem hiding this comment.
Tämä ei sisällä paljonkaan informaatiota, jos tämän haluaa lokittaa niin voisi laittaa debug-tasolla kaikilla parametreilla.
There was a problem hiding this comment.
Joo, nämä oli tarkoitus jättää vaan tilapäisesti, katselin lokitusten aikaleimoista miten pyynnöt etenee ja missä kohtaa kestää
| val kayttooikeusTunnisteet = | ||
| if (securityOperaatiot.onPaakayttaja()) Option.empty | ||
| else Option.apply(kantaOperaatiot.getKayttooikeusTunnisteet(securityOperaatiot.getKayttajanOikeudet().toSeq)) | ||
| LOG.info("haetaan käyttäjän käyttöoikeustunnisteet") |
There was a problem hiding this comment.
Tämä yleiskommenttina kaikista lokituksista. Suurin osa kantsii varmaan vaan poistaa ja loppuihin lisätä tunnisteet.
| getUsername: () => String = () => SecurityContextHolder.getContext.getAuthentication.getName(), | ||
| organisaatioClient: OrganisaatioService = OrganisaatioService, | ||
| kantaOperaatiot: KantaOperaatiot = new KantaOperaatiot(DbUtil.database), | ||
| optionalHttpSession: Option[HttpSession] = None) { |
There was a problem hiding this comment.
Miksi sessio on optionaalinen, eikö olisi yksinkertaisempaa vaatia se aina?
There was a problem hiding this comment.
En siinä kohti halunnut uusia joka kohtaa missä luodaan SecurityOperaatiot-intanssi niin oli helpompaa jättää se optionaaliseksi, kun ihan joka kohdassa missä sitä käytettiin ei tarvittu käyttöoikeuksia eikä siis myöskään sessiota vaan esim käyttäjän henkilö-oidin kysely. Ehkä yksinkertaistaisi jos sen passaisi silti aina.
| else | ||
| optionalHttpSession match { | ||
| case None => | ||
| LOG.warn("ei sessiota, oikkatunnisteet kannasta!") |
There was a problem hiding this comment.
Liittyen aikaisempaan kommenttiin, onko tämä skeenario jota halutaan/pakko tukea?
| case _ => | ||
| LOG.warn("oikkatunnisteet kannasta ja laitetaan sessioon!") | ||
| val tunnisteet = kantaOperaatiot.getKayttooikeusTunnisteet(kayttajanOikeudet.toSeq) | ||
| val uusinTunniste = kantaOperaatiot.getUusinKayttooikeusTunniste() |
There was a problem hiding this comment.
koska tunnisteiden ja uusimman tunnisteen haku ei tapahtu transaktionaalisesti, voi periaatteessa näiden kahden välissä kantaan tulla uusia tunnisteita, getKayttooikeusTunnisteet voisi palauttaa transaktionaalisesti tuplen.
…a on käytössä, tarkempi organisaation validointi viestirajapintaan
|
|
||
| @Test def testKayttajanOikeudet(): Unit = | ||
| @Test def testKayttajaSaaOikeudetLapsiorganisaatioihin(): Unit = | ||
| reset(mockOrganisaatioService, mockKantaoperaatiot) |
There was a problem hiding this comment.
Näissä testeissä olisi hyvä avata parilla lyhyellä kommentilla miten data alustetaan ja miksi ulos tulee mitä tulee
| @@ -352,7 +352,7 @@ object ViestiValidator: | |||
| virheet.incl("Metadata \"" + avain + "\": " + avainVirheet.mkString(",")) else virheet)) | |||
| .fold(l => l, r => r) | |||
|
|
|||
| val oidPattern: Regex = ("[0-9]+(\\.[0-9]+)+").r | |||
| val organisaatioOidPattern: Regex = "^1\\.2\\.246\\.562\\.(10|99)\\.\\d+$".r | |||
There was a problem hiding this comment.
VALIDATION_ORGANISAATIO_INVALID -tekstissä voisi selittää mikä on validi organisaatio nyt kun tuo patterni on monimutkaisempi
| /** | ||
| * Viestin haku | ||
| */ | ||
| @WithMockUser(value = "kayttaja", authorities = Array(SecurityConstants.SECURITY_ROOLI_LAHETYS_FULL, SecurityConstants.SECURITY_ROOLI_KATSELU_FULL, "ROLE_APP_HAKEMUS_CRUD")) |
There was a problem hiding this comment.
Näihin testeihin kaipaisin paria lyhyttä kommenttia jossa kerrotaan mitä tehdään ja miksi tapahtuu mitä tapahtuu.
…yhdenmukaiseen järjestykseen, paranneltu integraatiotestiä
…ajoitukset yhdenmukaisemmalla syntaksilla
No description provided.