review chgs nr. 12, incl: document idx_ private member

libirods_rule_engine_plugin-indexing.cpp

@@ -765,12 +765,20 @@ namespace {
 
     private:
 
+        //  Data structures used in tracking user/group permissions.
+
         std::map<std::string,std::string> users_{}, groups_{};
         std::map<std::string,std::list<std::string>> members_{};
         std::map<std::string,int> user_entry_{};
         std::multimap<int,std::string> user_perms_{}, group_perms_{};
         std::string owner_{};
 
+        //  idx_ is both:
+        //     - an index into the target permissions array (in the JSON) when filling that element.
+        //     - an internal flag which, when non-zero, means that the user permissions tracking data structures have been computed.
+        //  It is used by `reset_perms', thus also indirectly by `calc_perm_info', to determine the need to re-initialize those
+        //  structures before (re-)computation.
+
         int idx_{0};
 
         rsComm_t *conn{};
@@ -783,15 +791,6 @@ namespace {
 
         bool is_group( const std::string& gid ) {
             return groups_.find(gid) != groups_.end();
-		/*
-            try {
-                groups_.at(gid);
-            }
-            catch (const std::out_of_range&) {
-                return false;
-            }
-            return true;
-	    */
         }
 
         //  Is the given user a member of the given group ?
@@ -801,9 +800,8 @@ namespace {
                 const auto& user_list = members_.at(group_id);
                 return std::find( user_list.begin(), user_list.end(), user_id) != user_list.end();
             }
-            catch (...) {
-              std::cerr << ("unknown error\n");
-              throw;
+            catch (const std::out_of_range&) {
+                irods::log(LOG_ERROR, fmt::format("'{}' is not a group id", group_id));
             }
             return false;
         }
@@ -813,14 +811,14 @@ namespace {
         //  Helper method.  Reset the data structures that track existing permissions.
 
         void calc_user_info() {
-            irods::query q{conn, "select USER_GROUP_NAME,USER_GROUP_ID,USER_NAME,USER_ID"};
-            for (const auto & c:q) {
-                if (c[1] != c[3]) {
-                    members_[c[1]].push_back(c[3]);
-                    groups_[c[1]]=c[0];
+            irods::query q{ conn, "select USER_GROUP_NAME,USER_GROUP_ID,USER_NAME,USER_ID"};
+            for (const auto& row : q) {
+                if (row[1] != row[3]) {
+                    members_[row[1]].push_back(row[3]);
+                    groups_[row[1]]=row[0];
                 }
                 else {
-                    users_[c[3]]=c[2];
+                    users_[row[3]]=row[2];
                 }
             }
         }
@@ -851,7 +849,7 @@ namespace {
         //  Copy constructor, preserves user, group and is-a-member information, but resets other data structures
         //  in preparation for recomputing permissions info.
 
-        permissions_calculator(const permissions_calculator & x, rsComm_t *_conn)
+        permissions_calculator(const permissions_calculator& x, rsComm_t *_conn)
             : users_{x.users_}
             , groups_{x.groups_}
             , members_{x.members_}
@@ -864,7 +862,7 @@ namespace {
 
         void get_perms_list(nlohmann::json & j, const std::string & obj_id, const std::string & obj_type)
         {
-            calc_perm_info( obj_id, obj_type);
+            calc_perm_info( obj_id, obj_type); // idx_ member will be zero after this call.
 
             for (const auto & [pm,gid] : group_perms_) {
                 j["userPermissions"][idx_]["permission"] = perm_names.at(pm);
@@ -895,7 +893,7 @@ namespace {
     //  Calculate ownership and permissions for the object of the given ID
     //  Note obj_type should be either "DATA" or "COLL".
 
-    void permissions_calculator::calc_perm_info( const std::string & obj_id, const std::string & obj_type)
+    void permissions_calculator::calc_perm_info(const std::string& obj_id, const std::string& obj_type)
     {
         reset_perms();  // reset the variables used to calculate owner, user_perms and group_perms
                          //     for later conversion to JSON for indexing.
@@ -932,11 +930,11 @@ namespace {
                or higher privilege.
                */
         for (const auto & [uid,iperm] : user_entry_) {
-                bool include_user = true;
-                for (auto it = group_perms_.lower_bound(iperm); it!= group_perms_.end(); it++) {
-                        if (is_member_of(uid,it->second)) { include_user = false; break; }
-                }
-                if (include_user) { user_perms_.insert( make_pair(iperm, uid)); }
+            bool include_user = true;
+            for (auto it = group_perms_.lower_bound(iperm); it!= group_perms_.end(); it++) {
+                    if (is_member_of(uid,it->second)) { include_user = false; break; }
+            }
+            if (include_user) { user_perms_.insert( make_pair(iperm, uid)); }
         }
         // group_perms and user_perms are now ready for storing into the metadata index
     }