Skip to content

Commit 5d5375b

Browse files
d-w-moored-w-moore
committed
review changes nr. 3
1 parent a298592 commit 5d5375b

File tree

2 files changed

+93
-90
lines changed

2 files changed

+93
-90
lines changed

‎indexing_utilities.cpp

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -270,30 +270,33 @@ namespace irods {
270270
{
271271
}
272272

273-
idx_infos accum (const std::string &path)
273+
idx_infos accum(const std::string &path)
274274
{
275-
auto pos = path.find_last_of("/");
276-
if (path.size() < 1 || pos == std::string::npos)
275+
auto pos = (path.size() < 1 ? std::string::npos : path.find_last_of("/"));
276+
if (pos == std::string::npos) {
277277
throw path_format_error{ "Couldn't parse: "s + path };
278-
if (path == "/")
278+
}
279+
if (path == "/") {
279280
return calc("/");
280-
auto parent_len = (pos == 0) ? 1 :pos;
281-
auto st = accum(path.substr(0,parent_len));
282-
auto st_i = calc(path);
283-
st.insert (st_i.begin(), st_i.end());
284-
return st;
281+
}
282+
auto parent_path_length = (pos == 0) ? 1 :pos;
283+
auto accumulated_set = accum(path.substr(0,parent_path_length));
284+
auto current_path_set = calc(path);
285+
accumulated_set.insert (current_path_set.begin(), current_path_set.end());
286+
return accumulated_set;
285287
}
286288

287-
idx_infos calc(const std::string & p) { // Get individual contribution for last elem of path 'p'.
288-
// Use cached value if it exists.
289+
// Get individual contribution for last elem of path 'p'.
290+
// Use cached value if it exists.
291+
idx_infos calc(const std::string & path) {
289292
idx_infos i;
290293
try{
291-
i = m.at(p);
294+
i = m.at(path);
292295
}
293296
catch(const std::out_of_range &e) {
294-
i = (this->calc_)(p);
297+
i = (this->calc_)(path);
295298
}
296-
m[p] = i;
299+
m[path] = i;
297300
return i;
298301
}
299302

@@ -309,10 +312,10 @@ namespace irods {
309312
,index_type {index_type_}
310313
,index_tech {index_tech_}
311314
{
312-
}
313-
bool operator< (const index_info & other) const { return index_name < other.index_name ||
314-
index_type < other.index_type ||
315-
index_tech < other.index_tech; }
315+
}
316+
bool operator< (const index_info & other) const { return index_name < other.index_name ||
317+
index_type < other.index_type ||
318+
index_tech < other.index_tech; }
316319
std::string index_name;
317320
std::string index_type;
318321
std::string index_tech;
@@ -339,15 +342,15 @@ namespace irods {
339342
irods::query q {&comm,
340343
boost::str(boost::format("select META_COLL_ATTR_NAME,META_COLL_ATTR_VALUE,META_COLL_ATTR_UNITS where"
341344
" COLL_NAME = '%s' and META_COLL_ATTR_NAME = '%s'") % collname % config_.index)};
342-
for (const auto &row: q) {
345+
for (const auto &row : q) {
343346
std::string idx_name, idx_type, idx_tech;
344-
std::tie(idx_name, idx_type) = irods::indexing::parse_indexer_string (row[1]);
345-
idx_tech = row[2];
346-
s.insert ( { idx_name, idx_type, idx_tech } );
347+
std::tie(idx_name, idx_type) = irods::indexing::parse_indexer_string(row[1]);
348+
idx_tech = row[2];
349+
s.insert({idx_name, idx_type, idx_tech});
347350
}
348351
return s;
349352
};
350-
path_calc_and_cache <index_info> idx_info_cache { calculate_indexing_avus };
353+
path_calc_and_cache<index_info> idx_info_cache{ calculate_indexing_avus };
351354

352355
bool search_parent_tags = (_indexer.empty() && _index_name.empty());
353356

@@ -401,11 +404,11 @@ namespace irods {
401404

402405
struct query_failed : public std::runtime_error {
403406
query_failed( const std::string& e = "Query failed to fetch # of jobs active" )
404-
: std::runtime_error{e} {}
407+
: std::runtime_error{e} {}
405408
};
406409
struct job_limit_precision : public std::runtime_error {
407410
job_limit_precision( const std::string& e = "Job Limits may not exceed 32-bit unsigned integer precision" )
408-
: std::runtime_error{e} {}
411+
: std::runtime_error{e} {}
409412
};
410413

411414
try {
@@ -448,9 +451,9 @@ namespace irods {
448451
}
449452

450453
if ( search_parent_tags ) {
451-
//if (_index_type == "metadata" ) {
454+
// - ? need this guard ? //if (_index_type == "metadata" ) {
452455
auto info_set = idx_info_cache.accum( path.string() );
453-
for (auto info: info_set) {
456+
for (auto info : info_set) {
454457
schedule_policy_event_for_object(
455458
policy_name,
456459
path.string(),
@@ -464,9 +467,9 @@ namespace irods {
464467
{{ "job_category_tag", unique_key }} );
465468
++n_jobs;
466469
}
467-
//} else { throw std::runtime_error {"Parent tags search mode but index type is not 'metadata'"}; }
470+
// - ? ditto //} else { throw std::runtime_error {"Parent tags search mode but index type is not 'metadata'"}; }
468471
}
469-
else if ( ! (is_collection && _index_type == "full_text" )) { // full_text is meaningless for collection objects
472+
else if (!(is_collection && _index_type == "full_text")) { // full_text is meaningless for collection objects
470473
schedule_policy_event_for_object(
471474
policy_name,
472475
path.string(),
@@ -485,11 +488,11 @@ namespace irods {
485488
catch(const exception& _e) {
486489
rodsLog(
487490
LOG_ERROR,
488-
"failed to find indexing resource (error code=[%ld]) for object [%s]",static_cast<long>(_e.code()),
489-
path.string().c_str());
491+
"failed to find indexing resource (error code=[%ld]) for object [%s]", static_cast<long>(_e.code()),
492+
path.c_str());
490493
}
491494
catch (const std::runtime_error & e) {
492-
irods::log(LOG_ERROR,fmt::format("Abort indexing collection: {}",e.what()));
495+
irods::log(LOG_ERROR, fmt::format("Abort indexing collection: {}", e.what()));
493496
break;
494497
}
495498
if (iter != iter_end) { path = iter->path(); }
@@ -498,10 +501,10 @@ namespace irods {
498501
} // for path
499502
}
500503
catch( const path_format_error & f ) {
501-
rodsLog(LOG_ERROR , "path_format_error");
504+
rodsLog(LOG_ERROR, "path_format_error");
502505
}
503506
catch( const std::runtime_error & f ) {
504-
rodsLog(LOG_ERROR , "wrong index type");
507+
rodsLog(LOG_ERROR, "wrong index type");
505508
}
506509
} // schedule_policy_events_for_collection
507510

‎libirods_rule_engine_plugin-indexing.cpp

Lines changed: 56 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -435,24 +435,26 @@ namespace {
435435
idx.schedule_collection_operation(
436436
irods::indexing::operation_type::index,
437437
logical_path,
438-
_rei->rsComm->clientUser.userName
439-
, "::metadata" //value // -> Empty fields for index_name , index_type ,
440-
, "" //units // and index technology will signal that the collection
441-
); // operation should search upward in hierarchy for indexing tags
438+
_rei->rsComm->clientUser.userName,
439+
"::metadata", //value // -> Empty fields for index name, type, and technology
440+
"" //units // technology will signal that the collection operation
441+
); // should search upward in the hierarchy for indexing tags.
442442
}
443443
else {
444444
idx.schedule_metadata_indexing_event(
445-
logical_path,
446-
_rei->rsComm->clientUser.userName,
447-
"null" // attribute,
448-
"bb" // value,
449-
"cc" // units
445+
logical_path,
446+
_rei->rsComm->clientUser.userName,
447+
"null", // attribute, // -> the actual values are irrelevant now that we're using the
448+
"bb", // value, // NIEHS metadata schema
449+
"cc" // units
450450
);
451451
}
452452
}
453453
}
454-
catch(const std::exception &e){ const char* message = e.what();
455-
rodsLog(LOG_NOTICE,"Exception during pep_api_data_obj_unlink_post: %s",message); }
454+
catch(const std::exception &e) {
455+
const char* message = e.what();
456+
rodsLog(LOG_NOTICE,"Exception during pep_api_data_obj_unlink_post: %s",message);
457+
}
456458
}
457459
else if("pep_api_data_obj_unlink_post" == _rn) {
458460
auto it = _args.begin();
@@ -748,25 +750,23 @@ namespace {
748750

749751
private:
750752

751-
std::map<std::string,std::string> users {}
752-
, groups {};
753-
std::map<std::string,std::list<std::string>> members {};
754-
std::map<std::string,int> user_entry {};
755-
std::multimap<int,std::string> user_perms {}
756-
,group_perms {};
757-
std::string owner {};
753+
std::map<std::string,std::string> users_{}, groups_{};
754+
std::map<std::string,std::list<std::string>> members_{};
755+
std::map<std::string,int> user_entry_{};
756+
std::multimap<int,std::string> user_perms_{}, group_perms_{};
757+
std::string owner_{};
758758

759-
int idx {0};
759+
int idx_{0};
760760

761-
rsComm_t *Conn {};
761+
rsComm_t *conn{};
762762

763763
static std::map<int,std::string> perm_names;
764764

765765
public:
766766

767767
bool is_group( const std::string &gid ) {
768768
try {
769-
groups.at(gid);
769+
groups_.at(gid);
770770
}
771771
catch (const std::out_of_range&) {
772772
return false;
@@ -776,7 +776,7 @@ namespace {
776776

777777
bool is_member_of (const std::string& user_id, const std::string& group_id) {
778778
try {
779-
const auto & user_list = members.at(group_id);
779+
const auto& user_list = members_.at(group_id);
780780
return std::find( user_list.begin(), user_list.end(), user_id) != user_list.end();
781781
}
782782
catch (...) {
@@ -786,44 +786,44 @@ namespace {
786786
return false;
787787
}
788788

789-
void calc_perm_info( const std::string & obj_id, const std::string & obj_type);
789+
void calc_perm_info( const std::string& obj_id, const std::string& obj_type);
790790

791791
void calc_user_info() {
792-
irods::query q {Conn, "select USER_GROUP_NAME,USER_GROUP_ID,USER_NAME,USER_ID"};
792+
irods::query q{conn, "select USER_GROUP_NAME,USER_GROUP_ID,USER_NAME,USER_ID"};
793793
for (const auto & c:q) {
794794
if (c[1] != c[3]) {
795-
members[c[1]].push_back(c[3]);
796-
groups[c[1]]=c[0];
795+
members_[c[1]].push_back(c[3]);
796+
groups_[c[1]]=c[0];
797797
}
798798
else {
799-
users[c[3]]=c[2];
799+
users_[c[3]]=c[2];
800800
}
801801
}
802802
}
803803

804804
void reset_perms_ () {
805-
if (idx != 0) {
806-
user_entry = {};
807-
group_perms = {};
808-
user_perms = {};
809-
owner = {};
805+
if (idx_ != 0) {
806+
user_entry_ = {};
807+
group_perms_ = {};
808+
user_perms_ = {};
809+
owner_ = {};
810810
}
811-
idx = 0;
811+
idx_ = 0;
812812
}
813813

814814
public:
815815

816-
permissions_calculator(rsComm_t *Conn_)
817-
: Conn{Conn_}
816+
permissions_calculator(rsComm_t *_conn)
817+
: conn{_conn}
818818
{
819819
calc_user_info();
820820
}
821821

822-
permissions_calculator(const permissions_calculator & x, rsComm_t *Conn_)
823-
: users{x.users}
824-
, groups{x.groups}
825-
, members{x.members}
826-
, Conn{Conn_}
822+
permissions_calculator(const permissions_calculator & x, rsComm_t *_conn)
823+
: users_{x.users_}
824+
, groups_{x.groups_}
825+
, members_{x.members_}
826+
, conn{_conn}
827827
{
828828
reset_perms_();
829829
}
@@ -832,16 +832,16 @@ namespace {
832832
{
833833
calc_perm_info( obj_id, obj_type);
834834

835-
for (const auto & [pm,gid] : group_perms) {
836-
j["userPermissions"][idx]["permission"] = perm_names.at(pm);
837-
j["userPermissions"][idx++]["user"] = groups[gid];
835+
for (const auto & [pm,gid] : group_perms_) {
836+
j["userPermissions"][idx_]["permission"] = perm_names.at(pm);
837+
j["userPermissions"][idx_++]["user"] = groups_[gid];
838838
}
839-
for (const auto & [pm,uid] : user_perms) {
840-
j["userPermissions"][idx]["permission"] = perm_names.at(pm);
841-
j["userPermissions"][idx++]["user"] = users[uid];
839+
for (const auto & [pm,uid] : user_perms_) {
840+
j["userPermissions"][idx_]["permission"] = perm_names.at(pm);
841+
j["userPermissions"][idx_++]["user"] = users_[uid];
842842
}
843-
j["creator"] = owner;
844-
idx = -1; // force structures to be reinitialized on reuse
843+
j["creator"] = owner_;
844+
idx_ = -1; // force structures to be reinitialized on reuse
845845
}
846846
};
847847

@@ -859,26 +859,26 @@ namespace {
859859

860860
/* calculate:
861861
owner (aka the creator of the object) */
862-
irods::query qown { Conn, boost::str(boost::format("select %s_OWNER_NAME where %s_ID = '%s'")
862+
irods::query qown { conn, boost::str(boost::format("select %s_OWNER_NAME where %s_ID = '%s'")
863863
% obj_type % obj_type % obj_id ) };
864864
for (const auto & row : qown) {
865-
owner = row[0];
865+
owner_ = row[0];
866866
break;
867867
}
868868

869869
/* calculate:
870870
group_perms - maps the reported group permissions to the corresponding group IDs
871871
user_entry - maps the IDs of users which *not* groups to the permission levels reported for them
872872
*/
873-
irods::query qprm { Conn, boost::str(boost::format("select %s_ACCESS_TYPE,%s_ACCESS_USER_ID where %s_ID = '%s'")
873+
irods::query qprm { conn, boost::str(boost::format("select %s_ACCESS_TYPE,%s_ACCESS_USER_ID where %s_ID = '%s'")
874874
% obj_type % obj_type % obj_type % obj_id ) };
875875
for (const auto& i : qprm) {
876876
const auto iperm=std::stol(i[0]);
877877
if (is_group(i[1])) {
878-
group_perms.insert( make_pair(iperm, i[1]));
878+
group_perms_.insert(make_pair(iperm, i[1]));
879879
}
880880
else {
881-
user_entry[i[1]] = iperm;
881+
user_entry_[i[1]] = iperm;
882882
}
883883
}
884884

@@ -888,12 +888,12 @@ namespace {
888888
This is done by scanning group_perms to ensure each user in question is not a member of a group having equal
889889
or higher privilege.
890890
*/
891-
for (const auto & [uid,iperm] : user_entry) {
891+
for (const auto & [uid,iperm] : user_entry_) {
892892
bool include_user = true;
893-
for (auto it = group_perms.lower_bound(iperm); it!= group_perms.end(); it++) {
893+
for (auto it = group_perms_.lower_bound(iperm); it!= group_perms_.end(); it++) {
894894
if (is_member_of(uid,it->second)) { include_user = false; break; }
895895
}
896-
if (include_user) { user_perms.insert( make_pair(iperm, uid)); }
896+
if (include_user) { user_perms_.insert( make_pair(iperm, uid)); }
897897
}
898898
// group_perms and user_perms are now ready for storing into the metadata index
899899
}

0 commit comments

Comments
 (0)