Skip to content
This repository has been archived by the owner on Nov 23, 2021. It is now read-only.

security: create simple generator for constant class from XML #77

Closed
hohwille opened this issue Oct 8, 2014 · 13 comments
Closed

security: create simple generator for constant class from XML #77

hohwille opened this issue Oct 8, 2014 · 13 comments
Labels
Milestone

Comments

@hohwille
Copy link
Member

hohwille commented Oct 8, 2014

We should add a little tool to oasp4j-security that can generate a java class (type or interface) with the constants for all permissions from the security XML config (default location is classpath:config/app/security/access-control-schema.xml). The target package should be given as argument to the main method. Target directory should be "src/main/java" relative to current working directory by default. Optional parameters may be supplied to override the defaults ("--schema file://tmp/test.xml" or "--target foo/src/main/java" or "--classname MyPermissionConstants"). I hope this is easy enough to write the main method and arg-parsing by hand to avoid extra dependencies. Otherwise you could consider "mmm-cli".

So in our sample application you could call this:

io.oasp.module.security.common.base.PermissionGenerator --package io.gastronomy.restaurant.general.common.api

and get the permission constants generated in the file src/main/java/io/gastronomy/restaurant/general/common/api/PermissionConstants.java.

@hohwille hohwille added this to the oasp:1.0.0 milestone Oct 8, 2014
@maybeec
Copy link
Member

maybeec commented Oct 9, 2014

Might be also an issue for oasp/tools-cobigen. The only adaption, which has to be made is to enrich the cobigen-xmlplugin by an input reader to build the template model out of a xml file.

Maybe it is worth to go for this adaption rather than create a different tool?

@hohwille
Copy link
Member Author

Maybe it is worth to go for this adaption rather than create a different tool?

@may-bee makes sense but will we come to a solution soon then? Shall we work directly on cobigen to experiment with this? Working on XML in a generic way is typically done via XPath or XQuery. How would the current model of cobigen that is Java oriented fit here? Might be that we are making a simple problem complicated or might be a good idea that we need for cobigen anyway.

@maybeec
Copy link
Member

maybeec commented Oct 14, 2014

CobiGen itself only provides the framework to interconnect input reader, model builder and stuff like this. So it would be possible to easily provide two different plug-ins covering this needs:

  1. You can create a specialized input reader for the xml language you want to have and build a meaningful model out of it. This approach most fits to model driven development generation approaches, where models get very complex, I guess.
  2. You can also imagine the trivial appraoch, which builds up the model similarly to the input xml document structure. Thus you can use also XPath expressions using FreeMarker.

I would go for the second approach as the generation task we are talking about is of low complexity and thus we would enrich CobiGen by a generic xml input reader. I plan to have a planning meeting for the next issues on Friday. After that I can say more about the roadmap, but I would prioritize this issue a little bit higher than some existing ones.

@maybeec
Copy link
Member

maybeec commented Oct 21, 2014

I would go for this, but possibly in november, if this is ok for you?
We are currently working on the next release and I would go for another release of CobiGen at the beginning of december to release the CobiGen generator with the official oasp4j release.

@hohwille
Copy link
Member Author

Thanks for the update. We will solve this manually by hand-written code in the first place. Later we can then create a generator. This is no big deal for our simple sample app but a generator will be very helpful for maintenance of large apps. So November is fine. Manual changes have to make it into 1.0.0 release, generator is a nice-to-have addon.

@hohwille
Copy link
Member Author

io.oasp.gastronomy.restaurant.tablemanagement.logic.base.PermissionConstant

/oasp4j-example-application/src/main/resources/config/app/security/access-control-schema.xml

@hohwille
Copy link
Member Author

Some feedback from my end:

  • why so complicated with PermissionConstant per component? Why not a single class for the application? This would also ease the generation and usage with code completion.
  • why this strange naming schema? TABLEMANAGEMENT_GET_TABLES? We should always use singular form as we also do for REST to avoid unnecessary problems (some terms do not even have plural forms, etc. and in the example why is it GET_TABLES and REMOVE_TABLE?). My further suggestion is to use CamlCase instead of ugly UPPER_CASE.
  • Do we want to have the component as prefix? This makes it clear but also produces long names. As we have the general rule to avoid duplicated names the entity names shall be unique. Would it be sufficient to just name it GetTable? I think that would fit for KISS.

@maybeec
Copy link
Member

maybeec commented Oct 22, 2014

why so complicated with PermissionConstant per component? Why not a single class for the application? This would also ease the generation and usage with code completion.

For generation I am with you. For code completion, I do not see the difference as code completion will also be valid with multiple same named types. The only pitfall is that you get more results at the first time, when the PermissionConstant class has not been imported, yet.

why this strange naming schema? TABLEMANAGEMENT_GET_TABLES?

Because previously we had one PermissionConstant class for the whole application. Thus the naming schema had made sense. Now we should go for simple names as long we are sticking with the approach of declaring one PermissionConstant class per component.
In general I like this approach to distinguish permissions over different components, such that the range of a permission is clearly scoped.

My further suggestion is to use CamlCase instead of ugly UPPER_CASE

I think it is common sense to declare constants in Java using UPPER_CASE. Why do we want to break with this for this special type of constant class?

@hohwille
Copy link
Member Author

I think it is common sense to declare constants in Java using UPPER_CASE. Why do we want to break with this for this special type of constant class?

I was not talking about the constant but the name of the permission (in XML).

@maybeec
Copy link
Member

maybeec commented Oct 22, 2014

Ah ok, than I missunderstood your issue.
So for the security constants specified in XML, I would also go for CamelCase. But especially there it would be nice to have transparent naming conventions, such that you can easily see which group (resp. role) has access to which component and which operations of this component.
This knowledge might be helpful during development as it might indicate strange component scopes, e.g., if some groups (resp. roles) need permissions of nearly all components.

@arturk88
Copy link

Is someone allready writing the generator for the version 1.0.0? If not I will address this issue

@hohwille
Copy link
Member Author

From the discussion above that we already had we decided to use cobigen for this purpose. Cobigen has already been extended to support XML as input for Generation.

@maybeec
Copy link
Member

maybeec commented Dec 12, 2014

The new CobiGen release v1.2.0 will do the Job.
The only pitfall for this release is the necessity of defining the root package within the context.xml - line 61 & 66.
This is due to the information lack at this point, because CobIGen only handles the parsed xml, which does not indicate the target root package path.

There is an ongoing discussion on that in devonfw/cobigen#72.

@maybeec maybeec closed this as completed Dec 12, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants