Skip to content

Commit 9423b68

Browse files
shwstpprshwstppr
committed
tests
Signed-off-by: Abhishek Kumar <[email protected]>
1 parent 9f0d093 commit 9423b68

16 files changed

+2823
-130
lines changed

‎plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookApiServiceImpl.java‎

Lines changed: 50 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,25 @@ public ListResponse<WebhookResponse> listWebhooks(ListWebhooksCmd cmd) {
263263
final String name = cmd.getName();
264264
final String keyword = cmd.getKeyword();
265265
final String scopeStr = cmd.getScope();
266+
Webhook.Scope scope = null;
267+
if (StringUtils.isNotEmpty(scopeStr)) {
268+
scope = EnumUtils.getEnumIgnoreCase(Webhook.Scope.class, scopeStr);
269+
if (scope == null) {
270+
throw new InvalidParameterValueException("Invalid scope specified");
271+
}
272+
}
273+
if ((Webhook.Scope.Global.equals(scope) && !Account.Type.ADMIN.equals(caller.getType())) ||
274+
(Webhook.Scope.Domain.equals(scope) &&
275+
!List.of(Account.Type.ADMIN, Account.Type.DOMAIN_ADMIN).contains(caller.getType()))) {
276+
throw new InvalidParameterValueException(String.format("Scope %s can not be specified", scope));
277+
}
278+
Webhook.State state = null;
279+
if (StringUtils.isNotEmpty(stateStr)) {
280+
state = EnumUtils.getEnumIgnoreCase(Webhook.State.class, stateStr);
281+
if (state == null) {
282+
throw new InvalidParameterValueException("Invalid state specified");
283+
}
284+
}
266285
List<WebhookResponse> responsesList = new ArrayList<>();
267286
List<Long> permittedAccounts = new ArrayList<>();
268287
Ternary<Long, Boolean, Project.ListProjectResourcesCriteria> domainIdRecursiveListProject =
@@ -287,27 +306,6 @@ public ListResponse<WebhookResponse> listWebhooks(ListWebhooksCmd cmd) {
287306
SearchCriteria<WebhookJoinVO> sc = sb.create();
288307
accountManager.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts,
289308
listProjectResourcesCriteria);
290-
Webhook.Scope scope = null;
291-
if (StringUtils.isNotEmpty(scopeStr)) {
292-
try {
293-
scope = Webhook.Scope.valueOf(scopeStr);
294-
} catch (IllegalArgumentException iae) {
295-
throw new InvalidParameterValueException("Invalid scope specified");
296-
}
297-
}
298-
if ((Webhook.Scope.Global.equals(scope) && !Account.Type.ADMIN.equals(caller.getType())) ||
299-
(Webhook.Scope.Domain.equals(scope) &&
300-
!List.of(Account.Type.ADMIN, Account.Type.DOMAIN_ADMIN).contains(caller.getType()))) {
301-
throw new InvalidParameterValueException(String.format("Scope %s can not be specified", scope));
302-
}
303-
Webhook.State state = null;
304-
if (StringUtils.isNotEmpty(stateStr)) {
305-
try {
306-
state = Webhook.State.valueOf(stateStr);
307-
} catch (IllegalArgumentException iae) {
308-
throw new InvalidParameterValueException("Invalid state specified");
309-
}
310-
}
311309
if (scope != null) {
312310
sc.setParameters("scope", scope.name());
313311
}
@@ -345,9 +343,8 @@ public WebhookResponse createWebhook(CreateWebhookCmd cmd) throws CloudRuntimeEx
345343
final String stateStr = cmd.getState();
346344
Webhook.Scope scope = Webhook.Scope.Local;
347345
if (StringUtils.isNotEmpty(scopeStr)) {
348-
try {
349-
scope = Webhook.Scope.valueOf(scopeStr);
350-
} catch (IllegalArgumentException iae) {
346+
scope = EnumUtils.getEnumIgnoreCase(Webhook.Scope.class, scopeStr);
347+
if (scope == null) {
351348
throw new InvalidParameterValueException("Invalid scope specified");
352349
}
353350
}
@@ -359,9 +356,8 @@ public WebhookResponse createWebhook(CreateWebhookCmd cmd) throws CloudRuntimeEx
359356
}
360357
Webhook.State state = Webhook.State.Enabled;
361358
if (StringUtils.isNotEmpty(stateStr)) {
362-
try {
363-
state = Webhook.State.valueOf(stateStr);
364-
} catch (IllegalArgumentException iae) {
359+
state = EnumUtils.getEnumIgnoreCase(Webhook.State.class, stateStr);
360+
if (state == null) {
365361
throw new InvalidParameterValueException("Invalid state specified");
366362
}
367363
}
@@ -428,29 +424,27 @@ public WebhookResponse updateWebhook(UpdateWebhookCmd cmd) throws CloudRuntimeEx
428424
updateNeeded = true;
429425
}
430426
if (StringUtils.isNotEmpty(stateStr)) {
431-
try {
432-
Webhook.State state = Webhook.State.valueOf(stateStr);
433-
webhook.setState(state);
434-
updateNeeded = true;
435-
} catch (IllegalArgumentException iae) {
427+
Webhook.State state = EnumUtils.getEnumIgnoreCase(Webhook.State.class, stateStr);
428+
if (state == null) {
436429
throw new InvalidParameterValueException("Invalid state specified");
437430
}
431+
webhook.setState(state);
432+
updateNeeded = true;
438433
}
439434
Account owner = accountManager.getAccount(webhook.getAccountId());
440435
if (StringUtils.isNotEmpty(scopeStr)) {
441-
try {
442-
Webhook.Scope scope = Webhook.Scope.valueOf(scopeStr);
443-
if ((Webhook.Scope.Global.equals(scope) && !Account.Type.ADMIN.equals(owner.getType())) ||
444-
(Webhook.Scope.Domain.equals(scope) &&
445-
!List.of(Account.Type.ADMIN, Account.Type.DOMAIN_ADMIN).contains(owner.getType()))) {
446-
throw new InvalidParameterValueException(
447-
String.format("Scope %s can not be specified for owner %s", scope, owner.getName()));
448-
}
449-
webhook.setScope(scope);
450-
updateNeeded = true;
451-
} catch (IllegalArgumentException iae) {
436+
Webhook.Scope scope = EnumUtils.getEnumIgnoreCase(Webhook.Scope.class, scopeStr);
437+
if (scope == null) {
452438
throw new InvalidParameterValueException("Invalid scope specified");
453439
}
440+
if ((Webhook.Scope.Global.equals(scope) && !Account.Type.ADMIN.equals(owner.getType())) ||
441+
(Webhook.Scope.Domain.equals(scope) &&
442+
!List.of(Account.Type.ADMIN, Account.Type.DOMAIN_ADMIN).contains(owner.getType()))) {
443+
throw new InvalidParameterValueException(
444+
String.format("Scope %s can not be specified for owner %s", scope, owner.getName()));
445+
}
446+
webhook.setScope(scope);
447+
updateNeeded = true;
454448
}
455449
URI uri = URI.create(webhook.getPayloadUrl());
456450
if (StringUtils.isNotEmpty(payloadUrl)) {
@@ -490,8 +484,7 @@ public WebhookResponse createWebhookResponse(long webhookId) {
490484

491485
@Override
492486
public ListResponse<WebhookDeliveryResponse> listWebhookDeliveries(ListWebhookDeliveriesCmd cmd) {
493-
final CallContext ctx = CallContext.current();
494-
final Account caller = ctx.getCallingAccount();
487+
final Account caller = CallContext.current().getCallingAccount();
495488
final Long id = cmd.getId();
496489
final Long webhookId = cmd.getWebhookId();
497490
final Long managementServerId = cmd.getManagementServerId();
@@ -549,15 +542,15 @@ public WebhookDeliveryResponse executeWebhookDelivery(ExecuteWebhookDeliveryCmd
549542
final String secretKey = cmd.getSecretKey();
550543
final Boolean sslVerification = cmd.isSslVerification();
551544
final String payload = cmd.getPayload();
552-
final Account owner = accountManager.finalizeOwner(caller, null, null, null);
553545

554546
if (ObjectUtils.allNull(deliveryId, webhookId) && StringUtils.isBlank(payloadUrl)) {
555547
throw new InvalidParameterValueException(String.format("One of the %s, %s or %s must be specified",
556548
ApiConstants.ID, ApiConstants.WEBHOOK_ID, ApiConstants.PAYLOAD_URL));
557549
}
558-
if (deliveryId != null && webhookId != null) {
550+
if (deliveryId != null && (webhookId != null || StringUtils.isNotBlank(payloadUrl))) {
559551
throw new InvalidParameterValueException(
560-
String.format("Only one of the %s or %s can be specified", ApiConstants.ID, ApiConstants.WEBHOOK_ID));
552+
String.format("%s cannot be specified with %s or %s", ApiConstants.ID, ApiConstants.WEBHOOK_ID,
553+
ApiConstants.PAYLOAD_URL));
561554
}
562555
WebhookDeliveryVO existingDelivery = null;
563556
WebhookVO webhook = null;
@@ -590,7 +583,7 @@ public WebhookDeliveryResponse executeWebhookDelivery(ExecuteWebhookDeliveryCmd
590583
accountManager.checkAccess(caller, SecurityChecker.AccessType.OperateEntry, false, webhook);
591584
}
592585
if (ObjectUtils.allNull(deliveryId, webhookId)) {
593-
webhook = new WebhookVO(owner.getDomainId(), owner.getId(), payloadUrl, secretKey,
586+
webhook = new WebhookVO(caller.getDomainId(), caller.getId(), payloadUrl, secretKey,
594587
Boolean.TRUE.equals(sslVerification));
595588
}
596589
WebhookDelivery webhookDelivery = webhookService.executeWebhookDelivery(existingDelivery, webhook, payload);
@@ -649,7 +642,14 @@ public WebhookFilterResponse addWebhookFilter(AddWebhookFilterCmd cmd) throws Cl
649642
WebhookFilterVO webhookFilter = new WebhookFilterVO(webhook.getId(), type, mode, matchType, value);
650643
List<? extends WebhookFilter> existingFilters = webhookFilterDao.listByWebhook(webhook.getId());
651644
if (CollectionUtils.isNotEmpty(existingFilters)) {
652-
webhookFilter.isConflicting(existingFilters);
645+
WebhookFilter conflicting = webhookFilter.getConflicting(existingFilters);
646+
if (conflicting != null) {
647+
logger.error("Conflict detected when adding WebhookFilter having type: {}, mode: {}, " +
648+
"matchtype: {}, value: {} with existing {} for {}", type, mode, matchType, value, conflicting,
649+
webhook);
650+
throw new InvalidParameterValueException(String.format("Conflicting Webhook filter exists ID: %s",
651+
conflicting.getId()));
652+
}
653653
}
654654
webhookFilter = webhookFilterDao.persist(webhookFilter);
655655
webhookService.invalidateWebhookFiltersCache(webhook.getId());

‎plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookFilter.java‎

Lines changed: 16 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -45,50 +45,38 @@ enum MatchType {
4545
String getValue();
4646
Date getCreated();
4747

48-
static boolean overlaps(WebhookFilter.MatchType t1, String v1, WebhookFilter.MatchType t2, String v2) {
49-
switch (t1) {
48+
static boolean overlaps(WebhookFilter.MatchType oldMatchType, String oldValue, WebhookFilter.MatchType newMatchType, String newValue) {
49+
switch (oldMatchType) {
5050
case Exact:
51-
switch (t2) {
51+
switch (newMatchType) {
5252
case Exact:
53-
return v1.equals(v2);
54-
case Prefix:
55-
return v2.startsWith(v1 + ".");
56-
case Suffix:
57-
return v2.endsWith("." + v1);
58-
case Contains:
59-
return v2.contains(v1);
53+
return oldValue.equals(newValue);
6054
}
6155
break;
6256

6357
case Prefix:
64-
switch (t2) {
58+
switch (newMatchType) {
6559
case Exact:
6660
case Prefix:
67-
return v1.startsWith(v2 + ".") || v2.startsWith(v1 + ".");
68-
case Suffix:
69-
case Contains:
70-
return v1.contains(v2) || v2.contains(v1);
61+
return newValue.startsWith(oldValue);
7162
}
7263
break;
7364

7465
case Suffix:
75-
switch (t2) {
66+
switch (newMatchType) {
7667
case Exact:
77-
return v1.endsWith(v2) || v2.endsWith(v1);
78-
case Prefix:
7968
case Suffix:
80-
case Contains:
81-
return v1.contains(v2) || v2.contains(v1);
69+
return newValue.endsWith(oldValue);
8270
}
8371
break;
8472

8573
case Contains:
86-
switch (t2) {
74+
switch (newMatchType) {
8775
case Exact:
8876
case Prefix:
8977
case Suffix:
9078
case Contains:
91-
return v1.contains(v2) || v2.contains(v1);
79+
return newValue.contains(oldValue);
9280
}
9381
break;
9482

@@ -98,7 +86,7 @@ static boolean overlaps(WebhookFilter.MatchType t1, String v1, WebhookFilter.Mat
9886
return false;
9987
}
10088

101-
default boolean isConflicting(List<? extends WebhookFilter> existing) {
89+
default WebhookFilter getConflicting(List<? extends WebhookFilter> existing) {
10290
for (WebhookFilter f : existing) {
10391
if (f.getType() != this.getType()) {
10492
continue;
@@ -108,19 +96,20 @@ default boolean isConflicting(List<? extends WebhookFilter> existing) {
10896
if (f.getMode() == this.getMode()
10997
&& f.getMatchType() == this.getMatchType()
11098
&& f.getValue().equalsIgnoreCase(this.getValue())) {
111-
return true;
99+
return f;
112100
}
113101

114102
// 2. Opposite mode (INCLUDE vs EXCLUDE) — check for overlap
115-
if (f.getMode() != this.getMode()) {
103+
if (Mode.Exclude.equals(f.getMode())
104+
&& Mode.Include.equals(this.getMode())) {
116105
String oldVal = f.getValue().toUpperCase();
117106
String newVal = this.getValue().toUpperCase();
118107

119108
if (overlaps(f.getMatchType(), oldVal, this.getMatchType(), newVal)) {
120-
return true;
109+
return f;
121110
}
122111
}
123112
}
124-
return false;
113+
return null;
125114
}
126115
}

‎plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookServiceImpl.java‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.List;
2323
import java.util.Map;
2424
import java.util.UUID;
25+
import java.util.concurrent.CompletableFuture;
2526
import java.util.concurrent.ExecutionException;
2627
import java.util.concurrent.ExecutorService;
2728
import java.util.concurrent.Executors;
@@ -32,7 +33,6 @@
3233
import javax.naming.ConfigurationException;
3334

3435
import org.apache.cloudstack.acl.ControlledEntity;
35-
import org.apache.cloudstack.framework.async.AsyncCallFuture;
3636
import org.apache.cloudstack.framework.async.AsyncCallbackDispatcher;
3737
import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
3838
import org.apache.cloudstack.framework.async.AsyncRpcContext;
@@ -89,8 +89,8 @@ public class WebhookServiceImpl extends ManagerBase implements WebhookService, W
8989
@Inject
9090
AccountManager accountManager;
9191

92-
private LazyCache<org.apache.commons.lang3.tuple.Pair<Long, List<Long>>, List<WebhookVO>> webhooksCache;
93-
private LazyCache<Long, List<WebhookFilterVO>> webhookFiltersCache;
92+
protected LazyCache<org.apache.commons.lang3.tuple.Pair<Long, List<Long>>, List<WebhookVO>> webhooksCache;
93+
protected LazyCache<Long, List<WebhookFilterVO>> webhookFiltersCache;
9494

9595
protected WebhookDeliveryThread getDeliveryJob(Event event, Webhook webhook, Pair<Integer, Integer> configs) {
9696
WebhookDeliveryThread.WebhookDeliveryContext<WebhookDeliveryThread.WebhookDeliveryResult> context =
@@ -203,7 +203,7 @@ protected List<Runnable> getDeliveryJobs(Event event) throws EventBusException {
203203
}
204204

205205
protected Runnable getManualDeliveryJob(WebhookDelivery existingDelivery, Webhook webhook, String payload,
206-
AsyncCallFuture<WebhookDeliveryThread.WebhookDeliveryResult> future) {
206+
CompletableFuture<WebhookDeliveryThread.WebhookDeliveryResult> future) {
207207
if (StringUtils.isBlank(payload)) {
208208
payload = "{ \"CloudStack\": \"works!\" }";
209209
}
@@ -230,7 +230,7 @@ protected Runnable getManualDeliveryJob(WebhookDelivery existingDelivery, Webhoo
230230
event.setDescription(description);
231231
event.setResourceAccountUuid(resourceAccountUuid);
232232
ManualDeliveryContext<WebhookDeliveryThread.WebhookDeliveryResult> context =
233-
new ManualDeliveryContext<>(null, webhook, future);
233+
new ManualDeliveryContext<>(null, future);
234234
AsyncCallbackDispatcher<WebhookServiceImpl, WebhookDeliveryThread.WebhookDeliveryResult> caller =
235235
AsyncCallbackDispatcher.create(this);
236236
caller.setCallback(caller.getTarget().manualDeliveryCompleteCallback(null, null))
@@ -256,7 +256,7 @@ protected Void manualDeliveryCompleteCallback(
256256
AsyncCallbackDispatcher<WebhookServiceImpl, WebhookDeliveryThread.WebhookDeliveryResult> callback,
257257
ManualDeliveryContext<WebhookDeliveryThread.WebhookDeliveryResult> context) {
258258
WebhookDeliveryThread.WebhookDeliveryResult result = callback.getResult();
259-
context.future.complete(result);
259+
context.getFuture().complete(result);
260260
return null;
261261
}
262262

@@ -280,16 +280,20 @@ protected long cleanupOldWebhookDeliveries(long deliveriesLimit) {
280280
return processed;
281281
}
282282

283-
@Override
284-
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
283+
protected void initCaches() {
285284
webhooksCache = new LazyCache<>(
286-
16, 60,
285+
16, 60,
287286
(key) -> webhookDao.listByEnabledForDelivery(key.getLeft(), key.getRight())
288287
);
289288
webhookFiltersCache = new LazyCache<>(
290289
16, 60,
291290
(webhookId) -> webhookFilterDao.listByWebhook(webhookId)
292291
);
292+
}
293+
294+
@Override
295+
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
296+
initCaches();
293297
try {
294298
webhookJobExecutor = Executors.newFixedThreadPool(WebhookDeliveryThreadPoolSize.value(),
295299
new NamedThreadFactory(WEBHOOK_JOB_POOL_THREAD_PREFIX));
@@ -356,7 +360,7 @@ public void handleEvent(Event event) throws EventBusException {
356360
@Override
357361
public WebhookDelivery executeWebhookDelivery(WebhookDelivery delivery, Webhook webhook, String payload)
358362
throws CloudRuntimeException {
359-
AsyncCallFuture<WebhookDeliveryThread.WebhookDeliveryResult> future = new AsyncCallFuture<>();
363+
CompletableFuture<WebhookDeliveryThread.WebhookDeliveryResult> future = new CompletableFuture<>();
360364
Runnable job = getManualDeliveryJob(delivery, webhook, payload, future);
361365
webhookJobExecutor.submit(job);
362366
WebhookDeliveryThread.WebhookDeliveryResult result = null;
@@ -395,17 +399,18 @@ public List<Class<?>> getCommands() {
395399
return new ArrayList<>();
396400
}
397401

398-
static public class ManualDeliveryContext<T> extends AsyncRpcContext<T> {
399-
final Webhook webhook;
400-
final AsyncCallFuture<WebhookDeliveryThread.WebhookDeliveryResult> future;
402+
protected static class ManualDeliveryContext<T> extends AsyncRpcContext<T> {
403+
private final CompletableFuture<WebhookDeliveryThread.WebhookDeliveryResult> future;
401404

402-
public ManualDeliveryContext(AsyncCompletionCallback<T> callback, Webhook webhook,
403-
AsyncCallFuture<WebhookDeliveryThread.WebhookDeliveryResult> future) {
405+
public CompletableFuture<WebhookDeliveryThread.WebhookDeliveryResult> getFuture() {
406+
return future;
407+
}
408+
409+
public ManualDeliveryContext(AsyncCompletionCallback<T> callback,
410+
CompletableFuture<WebhookDeliveryThread.WebhookDeliveryResult> future) {
404411
super(callback);
405-
this.webhook = webhook;
406412
this.future = future;
407413
}
408-
409414
}
410415

411416
public class WebhookDeliveryCleanupWorker extends ManagedContextRunnable {

‎plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/dao/WebhookDaoImpl.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
4444
}
4545
@Override
4646
public List<WebhookVO> listByEnabledForDelivery(Long accountId, List<Long> domainIds) {
47-
logger.info("--------------------------------listByEnabledForDelivery accountId: {}, domainIds: {}", accountId, domainIds);
4847
SearchBuilder<WebhookVO> sb = createSearchBuilder();
4948
sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ);
5049
sb.and().op("scopeGlobal", sb.entity().getScope(), SearchCriteria.Op.EQ);

0 commit comments

Comments
 (0)