Issue 18077: Code improvements based on code review
authorIván Perdomo <ivan.perdomo@openbravo.com>
Wed, 03 Aug 2011 20:08:22 +0200
changeset 13415 f8c24d834286
parent 13414 bb1404edc6b4
child 13416 69b846d14162
Issue 18077: Code improvements based on code review
* Added a unique constraint for OpenID identifier
* Changed the module to be translatable
* Removed the silent update of the email when associating account. Will be ask
the user if he wants to update his email account
modules/org.openbravo.service.integration.openid/src-db/database/model/tables/OBSOID_USER_IDENTIFIER.xml
modules/org.openbravo.service.integration.openid/src-db/database/sourcedata/AD_MODULE.xml
modules/org.openbravo.service.integration.openid/src/org/openbravo/service/integration/openid/OpenIDManager.java
--- a/modules/org.openbravo.service.integration.openid/src-db/database/model/tables/OBSOID_USER_IDENTIFIER.xml	Wed Aug 03 20:05:52 2011 +0200
+++ b/modules/org.openbravo.service.integration.openid/src-db/database/model/tables/OBSOID_USER_IDENTIFIER.xml	Wed Aug 03 20:08:22 2011 +0200
@@ -50,6 +50,9 @@
       <foreign-key foreignTable="AD_USER" name="OBSOID_USER_OID_AD_USER">
         <reference local="AD_USER_ID" foreign="AD_USER_ID"/>
       </foreign-key>
+      <unique name="OBSOID_UN_IDENTIFIER">
+        <unique-column name="OPENID_IDENTIFIER"/>
+      </unique>
       <check name="OBSOID_USER_OID_ISACTIVE_CHECK"><![CDATA[ISACTIVE IN ('Y', 'N')]]></check>
     </table>
   </database>
--- a/modules/org.openbravo.service.integration.openid/src-db/database/sourcedata/AD_MODULE.xml	Wed Aug 03 20:05:52 2011 +0200
+++ b/modules/org.openbravo.service.integration.openid/src-db/database/sourcedata/AD_MODULE.xml	Wed Aug 03 20:08:22 2011 +0200
@@ -14,7 +14,7 @@
 <!--FF8080813141B198013141B86DD70003-->  <JAVAPACKAGE><![CDATA[org.openbravo.service.integration.openid]]></JAVAPACKAGE>
 <!--FF8080813141B198013141B86DD70003-->  <LICENSETYPE><![CDATA[OBPL]]></LICENSETYPE>
 <!--FF8080813141B198013141B86DD70003-->  <AUTHOR><![CDATA[Openbravo S.L.U.]]></AUTHOR>
-<!--FF8080813141B198013141B86DD70003-->  <ISTRANSLATIONREQUIRED><![CDATA[N]]></ISTRANSLATIONREQUIRED>
+<!--FF8080813141B198013141B86DD70003-->  <ISTRANSLATIONREQUIRED><![CDATA[Y]]></ISTRANSLATIONREQUIRED>
 <!--FF8080813141B198013141B86DD70003-->  <AD_LANGUAGE><![CDATA[en_US]]></AD_LANGUAGE>
 <!--FF8080813141B198013141B86DD70003-->  <HASCHARTOFACCOUNTS><![CDATA[N]]></HASCHARTOFACCOUNTS>
 <!--FF8080813141B198013141B86DD70003-->  <ISTRANSLATIONMODULE><![CDATA[N]]></ISTRANSLATIONMODULE>
--- a/modules/org.openbravo.service.integration.openid/src/org/openbravo/service/integration/openid/OpenIDManager.java	Wed Aug 03 20:05:52 2011 +0200
+++ b/modules/org.openbravo.service.integration.openid/src/org/openbravo/service/integration/openid/OpenIDManager.java	Wed Aug 03 20:08:22 2011 +0200
@@ -65,9 +65,9 @@
   private static ConsumerManager manager;
   private static OpenIDManager instance;
 
-  private static Logger log = Logger.getLogger(OpenIDManager.class);
+  private static final Logger log = Logger.getLogger(OpenIDManager.class);
 
-  private static final Map<String, DiscoveryInformation> discoveryInformationMap;
+  private static Map<String, DiscoveryInformation> discoveryInformationMap;
 
   public static final String ATTRIBUTE_EMAIL = "email";
   public static final String ATTRIBUTE_FIRSTNAME = "firstName";
@@ -75,13 +75,10 @@
 
   public static final String GOOGLE_OPENID_DISCOVER_URL = "https://www.google.com/accounts/o8/id";
 
-  static {
-    discoveryInformationMap = new HashMap<String, DiscoveryInformation>();
-  }
-
-  public static OpenIDManager getInstance() {
+  public static synchronized OpenIDManager getInstance() {
     if (instance == null) {
       instance = OBProvider.getInstance().get(OpenIDManager.class);
+      discoveryInformationMap = new HashMap<String, DiscoveryInformation>();
       manager = new ConsumerManager();
       manager.setAssociations(new InMemoryConsumerAssociationStore());
       manager.setNonceVerifier(new InMemoryNonceVerifier(5000));
@@ -150,34 +147,33 @@
       u = userCriteria.list().get(0).getUserContact();
     }
     return u;
+
   }
 
-  @SuppressWarnings("unchecked")
   public void associateAccount(Identifier oid, HttpServletRequest req, HttpServletResponse resp)
       throws Exception {
-    Map<String, String> userAttributes = (LinkedHashMap<String, String>) req
-        .getAttribute("attributes");
+    // Map<String, String> userAttributes = (LinkedHashMap<String, String>) req
+    // .getAttribute("attributes");
 
     User user = OBDal.getInstance().get(User.class, OBContext.getOBContext().getUser().getId());
-
-    if (!userAttributes.get(ATTRIBUTE_EMAIL).equals(user.getEmail())) {
-      try {
-        user.setEmail(userAttributes.get(ATTRIBUTE_EMAIL));
-        OBDal.getInstance().save(user);
-        OBDal.getInstance().flush();
-      } catch (Exception e) {
-        log.error("Error trying to update email for user: " + user.getUsername(), e);
-      }
-    }
+    // TODO: Ask the user if he wants to update the email account and uncomment this code
+    // if (!userAttributes.get(ATTRIBUTE_EMAIL).equals(user.getEmail())) {
+    // try {
+    // user.setEmail(userAttributes.get(ATTRIBUTE_EMAIL));
+    // OBDal.getInstance().save(user);
+    // OBDal.getInstance().flush();
+    // } catch (Exception e) {
+    // log.error("Error trying to update email for user: " + user.getUsername(), e);
+    // }
+    // }
 
     OBCriteria<OBSOIDUserIdentifier> oidCriteria = OBDal.getInstance().createCriteria(
         OBSOIDUserIdentifier.class);
     oidCriteria
         .add(Restrictions.eq(OBSOIDUserIdentifier.PROPERTY_OPENIDIDENTIFIER, oid.toString()));
-    oidCriteria.add(Restrictions.eq(OBSOIDUserIdentifier.PROPERTY_USERCONTACT, user));
 
     if (oidCriteria.count() > 0) {
-      log.warn("Account association already exists");
+      log.warn("Account association already exists - OpenID identifier: " + oid.toString());
       return;
     }
 
@@ -236,9 +232,6 @@
     if (authSuccess.hasExtension(AxMessage.OPENID_NS_AX)) {
       FetchResponse fetchResp = (FetchResponse) authSuccess.getExtension(AxMessage.OPENID_NS_AX);
 
-      // List emails = fetchResp.getAttributeValues("email");
-      // String email = (String) emails.get(0);
-
       List aliases = fetchResp.getAttributeAliases();
       Map attributes = new LinkedHashMap();
       for (Iterator iter = aliases.iterator(); iter.hasNext();) {