Skip to content

Commit e93c017

Browse files
kumaabkumaab
authored
RANGER-5134: Fix policy label processing in concurrent sessions (apache#545)
1 parent e32dfa0 commit e93c017

4 files changed

Lines changed: 60 additions & 31 deletions

File tree

‎security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6741,33 +6741,45 @@ public void run() {
67416741
getOrCreateLabel();
67426742
}
67436743

6744+
private boolean doesPolicyExist(XXPolicy xPolicy) {
6745+
return daoMgr.getXXPolicy().getById(xPolicy.getId()) != null;
6746+
}
6747+
67446748
private void getOrCreateLabel() {
67456749
LOG.debug("==> AssociatePolicyLabel.getOrCreateLabel(policyId={}, label={})", xPolicy.getId(), policyLabel);
67466750

6747-
XXPolicyLabel xxPolicyLabel = daoMgr.getXXPolicyLabels().findByName(policyLabel);
6748-
6749-
if (xxPolicyLabel == null) {
6750-
xxPolicyLabel = daoMgr.getXXPolicyLabels().findByName(policyLabel);
6751+
if (doesPolicyExist(xPolicy)) {
6752+
LOG.debug("Searching for policyLabel: {}", policyLabel);
6753+
XXPolicyLabel xxPolicyLabel = daoMgr.getXXPolicyLabels().findByName(policyLabel);
6754+
LOG.debug("Search returned: {}", xxPolicyLabel);
67516755

67526756
if (xxPolicyLabel == null) {
6757+
LOG.debug("Creating policyLabel: {}", policyLabel);
67536758
xxPolicyLabel = new XXPolicyLabel();
67546759

67556760
xxPolicyLabel.setPolicyLabel(policyLabel);
67566761

67576762
xxPolicyLabel = rangerAuditFields.populateAuditFieldsForCreate(xxPolicyLabel);
67586763
xxPolicyLabel = daoMgr.getXXPolicyLabels().create(xxPolicyLabel);
67596764
}
6760-
}
67616765

6762-
if (xxPolicyLabel != null) {
6763-
XXPolicyLabelMap xxPolicyLabelMap = new XXPolicyLabelMap();
6766+
// doing a find to check if the label is already associated with the policy (may happen in concurrent sessions)
6767+
List<XXPolicyLabelMap> xxPolicyLabelMapList = daoMgr.getXXPolicyLabelMap().findByPolicyIdAndLabelId(xPolicy.getId(), xxPolicyLabel.getId());
6768+
if (xxPolicyLabelMapList != null && !xxPolicyLabelMapList.isEmpty()) {
6769+
LOG.info("Policy with id {} already linked to label with id = {}", xPolicy.getId(), xxPolicyLabel.getId());
6770+
} else {
6771+
XXPolicyLabelMap xxPolicyLabelMap = new XXPolicyLabelMap();
67646772

6765-
xxPolicyLabelMap.setPolicyId(xPolicy.getId());
6766-
xxPolicyLabelMap.setPolicyLabelId(xxPolicyLabel.getId());
6773+
xxPolicyLabelMap.setPolicyId(xPolicy.getId());
6774+
xxPolicyLabelMap.setPolicyLabelId(xxPolicyLabel.getId());
67676775

6768-
xxPolicyLabelMap = rangerAuditFields.populateAuditFieldsForCreate(xxPolicyLabelMap);
6776+
xxPolicyLabelMap = rangerAuditFields.populateAuditFieldsForCreate(xxPolicyLabelMap);
67696777

6770-
daoMgr.getXXPolicyLabelMap().create(xxPolicyLabelMap);
6778+
LOG.debug("Creating a link for policy Id = {} to labelId = {}", xPolicy.getId(), xxPolicyLabel.getId());
6779+
daoMgr.getXXPolicyLabelMap().create(xxPolicyLabelMap);
6780+
}
6781+
} else {
6782+
LOG.info("Policy with id = {} does not exist, skipping to link label to the policy", xPolicy.getId());
67716783
}
67726784

67736785
LOG.debug("<== AssociatePolicyLabel.getOrCreateLabel(policyId={}, label={})", xPolicy.getId(), policyLabel);

‎security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -155,16 +155,17 @@ private void runRunnables(final List<Runnable> runnables, final boolean isParent
155155
LOG.debug("Executing {{}} runnables", runnables.size());
156156

157157
for (Runnable runnable : runnables) {
158-
boolean isThisTransactionCommitted = false;
158+
boolean isThisTransactionCommitted;
159159

160160
do {
161+
Object result = null;
161162
try {
162-
//Create new transaction
163+
//Create new transaction
163164
TransactionTemplate txTemplate = new TransactionTemplate(txManager);
164165

165166
txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW);
166167

167-
Object result = txTemplate.execute(status -> {
168+
result = txTemplate.execute(status -> {
168169
Object result1 = null;
169170

170171
LOG.debug("Executing runnable {{}}", runnable);
@@ -176,23 +177,13 @@ private void runRunnables(final List<Runnable> runnables, final boolean isParent
176177

177178
LOG.debug("executed runnable {}", runnable);
178179
} catch (OptimisticLockException optimisticLockException) {
179-
LOG.debug("Failed to execute runnable {}because of OpmimisticLockException", runnable);
180+
LOG.debug("Failed to execute runnable {} because of OptimisticLockException", runnable);
180181
} catch (Throwable e) {
181182
LOG.debug("Failed to execute runnable {}", runnable, e);
182183
}
183184

184185
return result1;
185186
});
186-
187-
isThisTransactionCommitted = result == runnable;
188-
189-
if (isParentTransactionCommitted) {
190-
if (!isThisTransactionCommitted) {
191-
LOG.info("Failed to commit runnable:[{}]. Will retry!", runnable);
192-
} else {
193-
LOG.debug("Committed runnable:[{}].", runnable);
194-
}
195-
}
196187
} catch (OptimisticLockException optimisticLockException) {
197188
if (LOG.isDebugEnabled()) {
198189
LOG.debug("Failed to commit TransactionService transaction for runnable:[{}]", runnable);
@@ -206,8 +197,17 @@ private void runRunnables(final List<Runnable> runnables, final boolean isParent
206197
LOG.debug("Failed to commit TransactionService transaction, throwable:[{}]", String.valueOf(e));
207198
}
208199
}
209-
}
210-
while (isParentTransactionCommitted && !isThisTransactionCommitted);
200+
201+
isThisTransactionCommitted = result == runnable;
202+
203+
if (isParentTransactionCommitted) {
204+
if (!isThisTransactionCommitted) {
205+
LOG.info("Failed to commit runnable:[{}]. Will retry!", runnable);
206+
} else {
207+
LOG.debug("Committed runnable:[{}].", runnable);
208+
}
209+
}
210+
} while (isParentTransactionCommitted && !isThisTransactionCommitted);
211211
}
212212
} else {
213213
LOG.debug("No runnables to execute");

‎security-admin/src/main/java/org/apache/ranger/db/XXPolicyLabelMapDao.java‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,27 @@ public List<XXPolicyLabelMap> findByPolicyId(Long policyId) {
4545
}
4646
}
4747

48-
public XXPolicyLabelMap findByPolicyLabelId(Long policyLabelId) {
48+
public List<XXPolicyLabelMap> findByPolicyLabelId(Long policyLabelId) {
4949
if (policyLabelId == null) {
5050
return null;
5151
}
5252

5353
try {
54-
return (XXPolicyLabelMap) getEntityManager().createNamedQuery("XXPolicyLabelMap.findByPolicyLabelId", tClass)
54+
return getEntityManager().createNamedQuery("XXPolicyLabelMap.findByPolicyLabelId", tClass)
55+
.setParameter("policyLabelId", policyLabelId).getResultList();
56+
} catch (NoResultException e) {
57+
return null;
58+
}
59+
}
60+
61+
public List<XXPolicyLabelMap> findByPolicyIdAndLabelId(Long policyId, Long policyLabelId) {
62+
if (policyId == null || policyLabelId == null) {
63+
return null;
64+
}
65+
66+
try {
67+
return getEntityManager().createNamedQuery("XXPolicyLabelMap.findByPolicyIdAndLabelId", tClass)
68+
.setParameter("policyId", policyId)
5569
.setParameter("policyLabelId", policyLabelId).getResultList();
5670
} catch (NoResultException e) {
5771
return null;

‎security-admin/src/main/resources/META-INF/jpa_named_queries.xml‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@
111111
</named-query>
112112

113113
<named-query name="XXPolicyLabelMap.findByPolicyLabelId">
114-
<query>SELECT obj FROM XXPolicyLabelMap obj WHERE obj.id = :policyLabelId
114+
<query>SELECT obj FROM XXPolicyLabelMap obj WHERE obj.policyLabelId = :policyLabelId
115115
</query>
116116
</named-query>
117117

@@ -120,7 +120,10 @@
120120
order by obj.policyId, obj.id
121121
</query>
122122
</named-query>
123-
123+
<named-query name="XXPolicyLabelMap.findByPolicyIdAndLabelId">
124+
<query>SELECT obj FROM XXPolicyLabelMap obj WHERE obj.policyLabelId = :policyLabelId AND obj.policyId = :policyId
125+
</query>
126+
</named-query>
124127

125128
<!-- XXPortalUserRole -->
126129
<named-query name="XXPortalUserRole.findByRoleUserId">

0 commit comments

Comments
 (0)