Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/master'
Browse files Browse the repository at this point in the history
  • Loading branch information
mederly committed Sep 27, 2023
2 parents a97e080 + 628cd21 commit e295b03
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,7 @@ protected void onPostProcessTarget(AjaxRequestTarget target) {
if (taskBean == null || WebComponentUtil.isClosedTask(taskBean.asObjectable())) {
stop(target);
taskOidForReloaded = null;
}
if (WebComponentUtil.isSuspendedTask(taskBean.asObjectable())) {
} else if (WebComponentUtil.isSuspendedTask(taskBean.asObjectable())) {
OperationResult taskResult = OperationResult.createOperationResult(taskBean.asObjectable().getResult());
if (taskResult != null && (taskResult.isFatalError() || taskResult.isPartialError())) {
stop(target);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.evolveum.midpoint.gui.impl.page.admin.resource.component.wizard.basic.ResourceTemplate.TemplateType;
import com.evolveum.midpoint.prism.PrismContext;
import com.evolveum.midpoint.prism.PrismObject;
import com.evolveum.midpoint.prism.query.ObjectPaging;
import com.evolveum.midpoint.prism.query.ObjectQuery;
import com.evolveum.midpoint.schema.GetOperationOptions;
import com.evolveum.midpoint.schema.SelectorOptions;
Expand Down Expand Up @@ -83,6 +84,16 @@ protected ObjectQuery getCustomizeContentQuery() {
return null;
}

@Override
protected ObjectPaging createPaging(long offset, long pageSize) {
if (type != null && TemplateType.CONNECTOR == type.getObject()) {
setSort("displayName", getDefaultSortOrder());
} else {
setSort(getDefaultSortParam(), getDefaultSortOrder());
}
return super.createPaging(offset, pageSize);
}

public TemplateTile<ResourceTemplate> createDataObjectWrapper(PrismObject<AssignmentHolderType> obj) {
if (obj.getCompileTimeClass().isAssignableFrom(ConnectorType.class)) {
@NotNull ConnectorType connectorObject = (ConnectorType) obj.asObjectable();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ private ScriptExpressionWrapper(ScriptExpressionEvaluatorType evaluator) {
}

public ScriptExpressionEvaluatorType toEvaluator() {
if (StringUtils.isEmpty(code)) {
return null;
}
return new ScriptExpressionEvaluatorType().code(code).language(language == null ? null : language.getLanguage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ public class SearchBoxConfigurationBuilder {
private Map<ItemPath, ItemDefinition<?>> availableDefinitions;
private SearchContext additionalSearchContext;

private boolean fullTextSearchEnabled;

public SearchBoxConfigurationBuilder type(Class<?> type) {
this.type = type;
return this;
Expand All @@ -108,6 +110,12 @@ public SearchBoxConfigurationBuilder availableDefinitions(Map<ItemPath, ItemDefi
this.availableDefinitions = availableDefinitions;
return this;
}

public SearchBoxConfigurationBuilder fullTextSearchEnabled(boolean fullTextSearchEnabled) {
this.fullTextSearchEnabled = fullTextSearchEnabled;
return this;
}

public SearchBoxConfigurationType create() {
SearchBoxConfigurationType defaultSearchBoxConfig = createDefaultSearchBoxConfig();

Expand All @@ -127,8 +135,13 @@ private SearchBoxConfigurationType createDefaultSearchBoxConfig() {
} else {
searchBoxConfig.getAllowedMode().addAll(Arrays.asList(SearchBoxModeType.BASIC, SearchBoxModeType.AXIOM_QUERY));
}
if (fullTextSearchEnabled && !searchBoxConfig.getAllowedMode().contains(SearchBoxModeType.FULLTEXT)) {
searchBoxConfig.getAllowedMode().add(SearchBoxModeType.FULLTEXT);
}
if (searchBoxConfig.getAllowedMode().size() == 1) {
searchBoxConfig.setDefaultMode(searchBoxConfig.getAllowedMode().iterator().next());
} else if (fullTextSearchEnabled) {
searchBoxConfig.setDefaultMode(SearchBoxModeType.FULLTEXT);
} else {
searchBoxConfig.setDefaultMode(SearchBoxModeType.BASIC);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ private SearchBoxConfigurationType getMergedConfiguration() {
.availableDefinitions(allSearchableItems)
.additionalSearchContext(additionalSearchContext)
.modelServiceLocator(modelServiceLocator)
.fullTextSearchEnabled(isFullTextSearchEnabled())
.create();
setDefaultSearchMode(defaultSearchBoxConfig);

return SearchConfigurationMerger.mergeConfigurations(defaultSearchBoxConfig, configuredSearchBox, modelServiceLocator);
}
Expand Down Expand Up @@ -295,31 +295,6 @@ private void sortItems(BasicQueryWrapper basicSearchWrapper) {
basicSearchWrapper.getItemsList().sort(Comparator.comparing(i -> i instanceof PropertySearchItemWrapper));
}

private void setDefaultSearchMode(SearchBoxConfigurationType config) {
List<SearchBoxModeType> allowedModes = config.getAllowedMode();

if (isFullTextSearchEnabled(type) && !allowedModes.contains(SearchBoxModeType.FULLTEXT)) {
allowedModes.add(SearchBoxModeType.FULLTEXT);
}

if (isFullTextSearchEnabled(type) && allowedModes.contains(SearchBoxModeType.FULLTEXT)) {
config.setDefaultMode(SearchBoxModeType.FULLTEXT);
return;
}
//This is not entirely correct. however, there is no clever options for now for handling fullText for assignments
// The main problem is the isFullTextSearchEnabled(type), since the fulltext for assignment is not actually for
// AssignmentType, but for its targetRef. So, fulltext for AbstractRoleType has to be specified
if (allowedModes.contains(SearchBoxModeType.FULLTEXT) && AssignmentType.class.isAssignableFrom(type) && isFullTextSearchEnabled((Class<C>) AbstractRoleType.class)) {
config.setDefaultMode(SearchBoxModeType.FULLTEXT);
return;
}
if (allowedModes.size() == 1) {
config.setDefaultMode(allowedModes.get(0));
return;
}
config.setDefaultMode(config.getDefaultMode());
}

private SearchBoxConfigurationType getConfiguredSearchBox() {
if (collectionView == null) {
return null;
Expand Down Expand Up @@ -352,6 +327,16 @@ public BasicQueryWrapper createDefaultSearchBoxConfigurationWrapper(SearchBoxCon
return searchConfigWrapper;
}

private boolean isFullTextSearchEnabled() {
//This is not entirely correct. however, there is no clever options for now for handling fullText for assignments
// The main problem is the isFullTextSearchEnabled(type), since the fulltext for assignment is not actually for
// AssignmentType, but for its targetRef. So, fulltext for AbstractRoleType has to be specified
if (AssignmentType.class.isAssignableFrom(type)) {
return isFullTextSearchEnabled((Class<C>) AbstractRoleType.class);
}
return isFullTextSearchEnabled(type);
}

private boolean isFullTextSearchEnabled(Class<C> type) {
OperationResult result = new OperationResult(LOAD_SYSTEM_CONFIGURATION);
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void auditEvent(@NotNull AuditEventType event, @Nullable ShadowType shado

auditRecord.setOutcome(clone.getStatus());

ObjectDeltaOperation<ShadowType> delta = createDelta(event, shadow, operationsWave, clone);
ObjectDeltaOperation<ShadowType> delta = createDelta(event, shadow, operationsWave, clone, ctx);
if (delta != null) {
auditRecord.addDelta(delta);
}
Expand Down Expand Up @@ -132,7 +132,12 @@ public void auditEvent(@NotNull AuditEventType event, @Nullable ShadowType shado
auditHelper.audit(auditRecord, nameResolver, ctx.getTask(), result);
}

private ObjectDeltaOperation<ShadowType> createDelta(AuditEventType event, ShadowType shadow, Collection<Operation> operations, OperationResult result) {
private ObjectDeltaOperation<ShadowType> createDelta(
AuditEventType event,
ShadowType shadow,
Collection<Operation> operations,
OperationResult result,
@NotNull ProvisioningContext ctx) {
if (shadow == null) {
return null;
}
Expand All @@ -158,7 +163,16 @@ private ObjectDeltaOperation<ShadowType> createDelta(AuditEventType event, Shado
}
}

return delta != null ? new ObjectDeltaOperation<>(delta, result) : null;
if (delta != null) {
ObjectDeltaOperation deltaOperation = new ObjectDeltaOperation<>(delta, result);
deltaOperation.setObjectName(object.getName());
if (ctx != null) {
deltaOperation.setResourceName(ctx.getResource().getName().toPolyString());
}
return deltaOperation;
}

return null;
}

private boolean isRecordResourceStageEnabled(SystemConfigurationType config) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;

/**
* Helps with the intra-cluster remote code execution.
Expand All @@ -60,8 +61,11 @@ public class ClusterExecutionHelperImpl implements ClusterExecutionHelper {
@Autowired private MidpointJsonProvider<?> jsonProvider;
@Autowired private MidpointYamlProvider<?> yamlProvider;

private Map<String, WebClient> clients = new ConcurrentHashMap<>();

private static final String DOT_CLASS = ClusterExecutionHelperImpl.class.getName() + ".";


@Override
public void execute(@NotNull ClientCode code, ClusterExecutionOptions options, String context, OperationResult parentResult) {
OperationResult result = parentResult.createSubresult(DOT_CLASS + "execute");
Expand Down Expand Up @@ -173,8 +177,9 @@ public OperationResult execute(@NotNull NodeType node, @NotNull ClientCode code,
if (isUpAndAlive || ClusterExecutionOptions.isTryAllNodes(options) ||
!isDead && ClusterExecutionOptions.isTryNodesInTransition(options)) {
try {
WebClient client = createClient(node, options, context);
WebClient client = getOrCreateClient(node, options, context);
if (client != null) {
resetClientForUse(client, options, context);
code.execute(client, node, result);
} else {
result.recordStatus(OperationResultStatus.NOT_APPLICABLE, "Node " + nodeIdentifier +
Expand All @@ -199,7 +204,30 @@ public OperationResult execute(@NotNull NodeType node, @NotNull ClientCode code,
return result;
}

private WebClient createClient(NodeType node, ClusterExecutionOptions options, String context) throws SchemaException {
/**
* Creates webclient if no webclient exists for specified node, otherwise reuses existing instance.
*
* Returned WebClient is not safe, and must be used under lock to prevent mixing URLs between calls.
*
* This limits use to one concurrent thread sending messages to one cluster node.
*
* The reason for reusing {@link WebClient} is that WebClient uses internal JVM HTTP Client whose
* smarts actually works against us.
*
* See MID-9106, but in short: WebClient creates HttpClient if used for first time. HttpClient also supports asynchronous use-cases,
* which means they do reference counting and WeakReferences to detect if client is unused and only then they free underlying threads.
*
* Before Java 21 this was only way to free allocated threads in HTTP Client (practicly wait till garbage collector marks
* this objects for garbage collection). But if WebClient was allocated in bursts, it was possible to reach thread count limit
* before any GC was able to kill outdated HTTP Clients.
*
* In Java 11+ there is option to shutdown HTTPClient manually, but that requires reflection and lowering JVM access protection
* which does not work correctly.
*
* So only doable solution is actually reuse WebClient (which requires synchronizing on it) to not allow for spawning large
* numbers of HTTPClients (+2 threads for each HTTPClient).
*/
private WebClient getOrCreateClient(NodeType node, ClusterExecutionOptions options, String context) throws SchemaException {
String baseUrl;
if (node.getUrl() != null) {
baseUrl = node.getUrl();
Expand All @@ -210,7 +238,22 @@ private WebClient createClient(NodeType node, ClusterExecutionOptions options, S

String url = baseUrl + "/ws/cluster";
LOGGER.debug("Going to execute '{}' on '{}'", context, url);
WebClient client = WebClient.create(url, Arrays.asList(xmlProvider, jsonProvider, yamlProvider));

WebClient client = clients.get(url);
if (client == null) {
client = WebClient.create(url, Arrays.asList(xmlProvider, jsonProvider, yamlProvider), true);
// Client was null, so we create it, but maybe other thread created also client, so we do putIfAbsent
// if previous return non-null, there is already existing client, so we should use this
// and let garbage collection take care of rest.
var previous = clients.putIfAbsent(url, client);
if (previous != null) {
client = previous;
}
}
return client;
}
private WebClient resetClientForUse(WebClient client, ClusterExecutionOptions options, String context) throws SchemaException {
client.reset();
if (!ClusterExecutionOptions.isSkipDefaultAccept(options)) {
client.accept(MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON, "application/yaml");
}
Expand Down

0 comments on commit e295b03

Please sign in to comment.