#622: implemented (added AccessControlConfig) #631
Conversation
…s code, provided example with JUnit)
There was a problem hiding this comment.
I like the idea of declaring perimission constants directly in Java. The XML approach was kind of an old design decision based on common experiences, which should be revised.
IMHO you can even deprecate the old stuff or even remove it if we go to EVE release. However, than the currently generated constants from the XML spec should be easily usable by the new approach with minor adaptions.
| AccessControlConfigSimple.PERMISSION_FIND_STAFF_MEMBER, AccessControlConfigSimple.PERMISSION_FIND_TABLE); | ||
| assertThat(readMasterData.getInherits()).isEmpty(); | ||
|
|
||
| // and when |
There was a problem hiding this comment.
"and when" Anti-pattern. Please seperate tests to let them as of just one reason (in theory).
There was a problem hiding this comment.
Sometimes being a little more pragmatic that the ideal theory is fine for me. I do not see any added value by separating this but I will do as it is faster done than further discussion...
There was a problem hiding this comment.
It is always easier to be someone's hero, than later teach him why you did it wrong and he should not.
| // then | ||
| assertThat(chief).isNotNull(); | ||
| assertThat(chief.getId()).isEqualTo(AccessControlConfigSimple.GROUP_CHIEF); | ||
| assertThat(flatten(chief.getPermissions())).containsExactly(AccessControlConfigSimple.PERMISSION_SAVE_OFFER, |
There was a problem hiding this comment.
Why do they have to be in order?
I think this should be InAnyOrder as well as below.
There was a problem hiding this comment.
I am fine with that and will change this accordingly.
|
@maybeec thanks for your review feedback. |
This PR implements #622:
AccessControlConfigthat implementsAccessControlProvider. It can therefore be used as a drop-in replacement foraccess-control-schema.xmlandAccessControlProviderImpl.Feedback to improve the config "API" is most welcome.
I did not (yet) deprecate the code that may not be used anymore such as
AccessControlProviderImplandAccessControlSchema, etc.First I want to await feedback from others but I am quite convinced that the old XML stuff is kind of odd compared to the new approach. We still have issues with referencing and code-generation from the XML. All solved as a single source of truth with the new approach where everything is in one place.