Property "updateUsers" is another thing unrelated to Root user password setup. It's mentioned in this JIRA https://issues.jboss.org/browse/GTNPORTAL-3296 . It would be nice if OrganizationDatabaseInitializer from eXo core doesn't have all methods private, so we don't need to fork whole class into GateIn if some minor change is required. I can create JIRA to eXo core for this and also for "updateUsers" support. WDYT?
On Mon, Dec 9, 2013 at 4:44 PM, Lucas Ponce <lponce@redhat.com> wrote:
Hello Hai,
Sorry for my delay, it was public holiday past friday and I coudn't comment until now.
Some comments in-line:
----- Mensaje original -----
> De: "Hai Nguyen" <haint@exoplatform.com>
> Para: "gatein-dev" <gatein-dev@lists.jboss.org>
> Enviados: Jueves, 5 de Diciembre 2013 10:55:14
> Asunto: [gatein-dev] Setup Root password feature improvement
>This feature can be disable removing PortalSetupFilter from portal.war/WEB-INF/web.xml and replacing SetupOrganizationDatabaseInitializer per OrganizationDatabaseInitializer in organization-configuration.xml.
> Hi guys,
>
> Currently, the setup root password feature has some disadvantages.
> Those are:
> - This feature depended directly on portal core and could not enable/disable
> feature
But we aim to not modify web.xml because products would not modify directly portal.war (thank to extension mechanism).> - The SetupOrganizationDatabaseInitializer was clone ofYes, I agree that clone was not more elegant way, but there was not possibility to extend OrganizationDatabaseInitializer with a custom class, so clone was the only option to add custom behaviour.
> OrganizationDatabaseInitializer (in core service), that is hard for maintain
I would suggest to give the ability to extend this class and override logic from other localization rather that exo.core module.
IMO, the new behaviors are not much to introduce a new custom class. Beside, I don't know why we need introduce more "updateUsers" property that is not necessary. If you looking for my PR, you will see we don't need SetupOrganizationDatabaseInitializer for this feature
> - The implementation of PortalSetupService is not really good because it usedEvery code always can be improved, for sure.
> many static methods and hardcode properties
Some comments about static methods:
- There is a static variable that is used to provide a flag about if setup is complete (isSetup()) method. Methods that works with this static variable are also static.
- This class also can be used from a portal-setup.sh script, so for that particular scenario are also used static methods.
About properties, I agree that they can be read it from a configuration.properties file, but at time we coded it, we didn't see a clear scenario for change, so that was main reason to not change it, but I agree that those properties can be also read it from a file.
Take a look my change. Actually, the PortalSetupService needs only static method that is "encodePassword(String plainTextPassword)" for PortalSetupCommand. The PortalSetupService should respect normal GateIn service that means it should be declared and configured in a configuration xml file.This issue is a expected behaviour as password should be encoded using portal-setup.sh/.bat script.
No, this issue relates to JCR session. We could fix it by RequestLifeCycle.begin/end().It was fixed in my PR, of course> We have collected issues of this feature and createdI don't see bugs related to this module.
> https://issues.jboss.org/browse/GTNPORTAL-3315 to improve it
>
> Who can review or have idea about that?
>
I agree that we should write a better note about it, sorry for that.
I can understand that may be we can unify criterion about where to place filters or if all properties have to be read from files.Regards,
Lucas
If you have time, could you help me verify my PR doesn't have any impact for this module?Thanks a lot,> Kind regards,
>
> _______________________________________________
> gatein-dev mailing list
> gatein-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/gatein-dev
_______________________________________________ gatein-dev mailing list gatein-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/gatein-dev