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

First review #1

Closed
CucumisSativus opened this issue May 17, 2019 · 2 comments
Closed

First review #1

CucumisSativus opened this issue May 17, 2019 · 2 comments

Comments

@CucumisSativus
Copy link
Collaborator

  1. where are tests? :D

  2. PasswordHasherJavaxCrypto

  • factory IMO should be create once (need to check it)
  1. UserStore
  • create should be indepotent
  • delete should delete permissions as well
  • I would use uuid as id, but no strong feelings there
    Capabilities
  • I would treat them more as a separate concern (separate table, with user ID and capability IMO I
    would use pstgresql enum there
  • as for component this tree like structure and handling by spliting strings is a bit fishy, not
    sure if I have a better idea already
  1. Component:
  • There must be uniqueness check, to make sure that no unexpected access is done
  • IMO child should accept component, not string
  • how about keeping information of the absolute path of the component with some indication of root
    (IMO something like in linux file system /top/less/something-else) IMO its less errorprone
  1. Identified:
  • completely dont get the whole idea there
@esarbe
Copy link
Owner

esarbe commented May 18, 2019

  1. My dog ate them. No! I forgot them on the train, I mean, the one-armed man took them and ... I'm working on them...

  2. Absolutely. Fixed that.

  • idempotent create: that's correct. Edit: fixed that.
  • indeed, that needs to be fixed. Edit: done.
  • i'd like to expose UUIDs as stable, external identifiers and longs as internal identifiers. I've yet to implement that. Identified was a failed experiment in that direction. Edit: Created Expose Entities with UUID #2 for that.
  • Capabilities as enums: I'm a bit unsure about that. Beyond basic capabilities like 'Read' and so on I'd like the components/modules/service/domains to be able to expose specialized capabilities. that could get difficult with enums.
  • Components: see below
  1. Components
  • I don't quite get the uniqueness check. can you explain?
  • see below
  • I've slightly changed the implementation to hopefully improve the naming and use, but I don't have come up with better idea of how t implement it or with a better concept. Any help is appreciated!
  1. Identified is an aborted effort separate an entity from the id and still be able to 'materialize' the id the required type. I've put it into a branch and will revisit it at a later point. for now I can just create and expose UUIDs for the entities that require them. (tbd)

Thanks for the review! Much appreciated.

@esarbe
Copy link
Owner

esarbe commented Jun 19, 2019

Provided some further tests with #7

esarbe pushed a commit that referenced this issue Jan 31, 2022
An initial attempt at offering a classification (entry, final, plenary)
for votes.

This implements issue #1.
@esarbe esarbe closed this as completed in 053af1d Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants