-
Notifications
You must be signed in to change notification settings - Fork 6
[NAE-2147] Merge 6.5.0 into 7.0.0 #331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
af2f5cb
ea2bc25
d02feea
458b89e
c61be81
8bf002e
c7cb0d2
16a3cd6
12c6379
860b0d0
255340f
2bde47c
570a7ad
6c71bdb
06d484d
a5e0350
1097ea3
9dc653e
dd06c6e
7971f6d
817cb99
be3da23
e3f80ab
145e3e0
1c16250
7211660
fa81b02
4940da2
c550d27
9cebf5f
c47db8d
32c8a4b
56d016a
afe468f
438bb1d
32f4c1a
9606298
1fa70e9
282497a
34657bb
7f14cf6
d6cec3e
ea16ee2
103ad7b
e51f1f6
77a175a
73fb713
318c517
8478fdb
8875df0
1e41d46
693438c
1c98631
7817b3e
1171c1d
c6febb8
9866500
76b93bd
cef4f14
0be42bf
f0dded0
bcbb9a1
082883b
36abe63
2a2c35f
f6dedf0
41dacdc
7a85f22
608c86d
490cbb7
f1981c5
84dac05
7ec0cc9
a65b028
33ff704
4832d9d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,13 +1,39 @@ | ||||||||||||||||||||||||||
| package com.netgrif.application.engine.elastic.service; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import co.elastic.clients.elasticsearch._types.query_dsl.BoolQuery; | ||||||||||||||||||||||||||
| import co.elastic.clients.elasticsearch._types.query_dsl.QueryStringQuery; | ||||||||||||||||||||||||||
| import com.netgrif.application.engine.configuration.ElasticsearchConfiguration; | ||||||||||||||||||||||||||
| import com.netgrif.application.engine.elastic.domain.ElasticPetriNetRepository; | ||||||||||||||||||||||||||
| import com.netgrif.application.engine.elastic.service.executors.Executor; | ||||||||||||||||||||||||||
| import com.netgrif.application.engine.elastic.service.interfaces.IElasticPetriNetService; | ||||||||||||||||||||||||||
| import com.netgrif.application.engine.objects.auth.domain.LoggedUser; | ||||||||||||||||||||||||||
| import com.netgrif.application.engine.objects.elastic.domain.ElasticPetriNet; | ||||||||||||||||||||||||||
| import com.netgrif.application.engine.objects.petrinet.domain.PetriNet; | ||||||||||||||||||||||||||
| import com.netgrif.application.engine.objects.petrinet.domain.PetriNetSearch; | ||||||||||||||||||||||||||
| import com.netgrif.application.engine.petrinet.service.interfaces.IPetriNetService; | ||||||||||||||||||||||||||
| import com.netgrif.application.engine.petrinet.web.responsebodies.PetriNetReference; | ||||||||||||||||||||||||||
| import com.netgrif.application.engine.utils.FullPageRequest; | ||||||||||||||||||||||||||
| import lombok.extern.slf4j.Slf4j; | ||||||||||||||||||||||||||
| import org.springframework.beans.factory.annotation.Autowired; | ||||||||||||||||||||||||||
| import org.springframework.context.annotation.Lazy; | ||||||||||||||||||||||||||
| import org.springframework.dao.InvalidDataAccessApiUsageException; | ||||||||||||||||||||||||||
|
Comment on lines
+18
to
19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify if @lazy annotation is necessary. The @lazy annotation on line 19 suggests potential circular dependency issues. Consider verifying if this is still needed after fixing the constructor injection. 🏁 Script executed: #!/bin/bash
# Check for circular dependencies by finding where ElasticPetriNetService is injected
rg -A 5 "ElasticPetriNetService" --glob "*.java" | grep -E "@Autowired|constructor|@RequiredArgsConstructor"Length of output: 457 🏁 Script executed: #!/bin/bash
# Show the beginning of ElasticPetriNetService to see where and how @Lazy is applied
sed -n '1,200p' application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticPetriNetService.javaLength of output: 9587 Remove unused Lazy import The import
🤖 Prompt for AI Agents
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The annotation is presnet at line 58. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||||||||||||||
| import org.springframework.data.domain.Page; | ||||||||||||||||||||||||||
| import org.springframework.data.domain.PageImpl; | ||||||||||||||||||||||||||
| import org.springframework.data.domain.Pageable; | ||||||||||||||||||||||||||
| import org.springframework.data.elasticsearch.client.elc.ElasticsearchTemplate; | ||||||||||||||||||||||||||
| import org.springframework.data.elasticsearch.client.elc.NativeQuery; | ||||||||||||||||||||||||||
| import org.springframework.data.elasticsearch.client.elc.NativeQueryBuilder; | ||||||||||||||||||||||||||
| import org.springframework.data.elasticsearch.core.SearchHitSupport; | ||||||||||||||||||||||||||
| import org.springframework.data.elasticsearch.core.SearchHits; | ||||||||||||||||||||||||||
| import org.springframework.data.elasticsearch.core.mapping.IndexCoordinates; | ||||||||||||||||||||||||||
| import org.springframework.stereotype.Service; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import java.util.*; | ||||||||||||||||||||||||||
| import java.util.function.BinaryOperator; | ||||||||||||||||||||||||||
| import java.util.stream.Collectors; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import static org.springframework.data.elasticsearch.client.elc.Queries.termQuery; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| @Service | ||||||||||||||||||||||||||
| @Slf4j | ||||||||||||||||||||||||||
| public class ElasticPetriNetService implements IElasticPetriNetService { | ||||||||||||||||||||||||||
|
|
@@ -16,9 +42,23 @@ public class ElasticPetriNetService implements IElasticPetriNetService { | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private final Executor executors; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public ElasticPetriNetService(ElasticPetriNetRepository repository, Executor executors) { | ||||||||||||||||||||||||||
| private IPetriNetService petriNetService; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
Comment on lines
+45
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add petriNetService to constructor injection. The - private IPetriNetService petriNetService;
+ private final IPetriNetService petriNetService;
- public ElasticPetriNetService(ElasticPetriNetRepository repository, Executor executors, ElasticsearchTemplate template, ElasticsearchConfiguration elasticsearchConfiguration) {
+ public ElasticPetriNetService(ElasticPetriNetRepository repository, Executor executors, ElasticsearchTemplate template, ElasticsearchConfiguration elasticsearchConfiguration, IPetriNetService petriNetService) {
this.repository = repository;
this.executors = executors;
this.template = template;
this.elasticsearchConfiguration = elasticsearchConfiguration;
+ this.petriNetService = petriNetService;
}Also applies to: 51-56 🤖 Prompt for AI Agents
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. petriNetService properties is initialized with lazy setter at line 60. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tuplle Thank you for the clarification! You're absolutely right. I can see that @Lazy
@Autowired
public void setPetriNetService(IPetriNetService petriNetService) {
this.petriNetService = petriNetService;
}This is a valid Spring dependency injection pattern, often used to resolve circular dependencies. The learns something new ✏️ Learnings added
|
||||||||||||||||||||||||||
| private final ElasticsearchTemplate template; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| protected ElasticsearchConfiguration elasticsearchConfiguration; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public ElasticPetriNetService(ElasticPetriNetRepository repository, Executor executors, ElasticsearchTemplate template, ElasticsearchConfiguration elasticsearchConfiguration) { | ||||||||||||||||||||||||||
| this.repository = repository; | ||||||||||||||||||||||||||
| this.executors = executors; | ||||||||||||||||||||||||||
| this.template = template; | ||||||||||||||||||||||||||
| this.elasticsearchConfiguration = elasticsearchConfiguration; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| @Lazy | ||||||||||||||||||||||||||
| @Autowired | ||||||||||||||||||||||||||
| public void setPetriNetService(IPetriNetService petriNetService) { | ||||||||||||||||||||||||||
| this.petriNetService = petriNetService; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||
|
|
@@ -54,4 +94,111 @@ public void remove(String id) { | |||||||||||||||||||||||||
| log.info("[" + id + "]: PetriNet \"" + id + "\" deleted"); | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * Method for search of PetriNets in Elastic | ||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||
| * @param requests - search body, for now only title working | ||||||||||||||||||||||||||
| * @param user - logged user | ||||||||||||||||||||||||||
| * @param pageable - pageable for paging | ||||||||||||||||||||||||||
| * @param locale - internacionalization | ||||||||||||||||||||||||||
| * @param isIntersection - property for merging filter, not implemented now, use false | ||||||||||||||||||||||||||
| * @return Page<PetriNetReference> - page of PetriNetReferences | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||
| public Page<PetriNetReference> search(PetriNetSearch requests, LoggedUser user, Pageable pageable, Locale locale, Boolean isIntersection) { | ||||||||||||||||||||||||||
| if (requests == null) { | ||||||||||||||||||||||||||
| throw new IllegalArgumentException("Request can not be null!"); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| log.debug("Searching for PetriNet query with logged user [{}]", user.getId()); | ||||||||||||||||||||||||||
| // TODO: impersonation | ||||||||||||||||||||||||||
| // LoggedUser loggedOrImpersonated = user.getSelfOrImpersonated(); | ||||||||||||||||||||||||||
| LoggedUser loggedOrImpersonated = user; | ||||||||||||||||||||||||||
| NativeQuery query = buildQuery(requests, loggedOrImpersonated, pageable, locale, isIntersection); | ||||||||||||||||||||||||||
| List<PetriNet> netPage; | ||||||||||||||||||||||||||
| long total; | ||||||||||||||||||||||||||
| if (query != null) { | ||||||||||||||||||||||||||
| SearchHits<ElasticPetriNet> hits = template.search(query, ElasticPetriNet.class, IndexCoordinates.of(elasticsearchConfiguration.elasticPetriNetIndex())); | ||||||||||||||||||||||||||
| Page<ElasticPetriNet> indexedNets = (Page) SearchHitSupport.unwrapSearchHits(SearchHitSupport.searchPageFor(hits, query.getPageable())); | ||||||||||||||||||||||||||
| netPage = petriNetService.findAllById(indexedNets.get().map(ElasticPetriNet::getStringId).collect(Collectors.toList())); | ||||||||||||||||||||||||||
| total = indexedNets.getTotalElements(); | ||||||||||||||||||||||||||
| log.debug("Found [{}] total elements of page [{}]", netPage.size(), pageable.getPageNumber()); | ||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||
| netPage = Collections.emptyList(); | ||||||||||||||||||||||||||
| total = 0; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return new PageImpl<>(netPage.stream().map(net -> new PetriNetReference(net, locale)).collect(Collectors.toList()), pageable, total); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| protected NativeQuery buildQuery(PetriNetSearch request, LoggedUser user, Pageable pageable, Locale locale, Boolean isIntersection) { | ||||||||||||||||||||||||||
| List<BoolQuery.Builder> singleQueries = new LinkedList<>(); | ||||||||||||||||||||||||||
| singleQueries.add(buildSingleQuery(request, user, locale)); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (isIntersection && !singleQueries.stream().allMatch(Objects::nonNull)) { | ||||||||||||||||||||||||||
| // one of the queries evaluates to empty set => the entire result is an empty set | ||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||
| } else if (!isIntersection) { | ||||||||||||||||||||||||||
| singleQueries = singleQueries.stream().filter(Objects::nonNull).collect(Collectors.toList()); | ||||||||||||||||||||||||||
| if (singleQueries.isEmpty()) { | ||||||||||||||||||||||||||
| // all queries result in an empty set => the entire result is an empty set | ||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| BinaryOperator<BoolQuery.Builder> reductionOperation = isIntersection ? (a, b) -> a.must(b.build()._toQuery()) : (a, b) -> a.should(b.build()._toQuery()); | ||||||||||||||||||||||||||
| BoolQuery.Builder query = singleQueries.stream().reduce(new BoolQuery.Builder(), reductionOperation); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| NativeQueryBuilder builder = new NativeQueryBuilder(); | ||||||||||||||||||||||||||
| return builder | ||||||||||||||||||||||||||
| .withQuery(query.build()._toQuery()) | ||||||||||||||||||||||||||
| .withPageable(pageable) | ||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| protected BoolQuery.Builder buildSingleQuery(PetriNetSearch request, LoggedUser user, Locale locale) { | ||||||||||||||||||||||||||
| BoolQuery.Builder query = new BoolQuery.Builder(); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| buildFullTextQuery(request, query); | ||||||||||||||||||||||||||
| boolean resultAlwaysEmpty = buildGroupQuery(request, user, locale, query); | ||||||||||||||||||||||||||
| if (resultAlwaysEmpty) { | ||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| return query; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| protected void buildFullTextQuery(PetriNetSearch request, BoolQuery.Builder query) { | ||||||||||||||||||||||||||
| if (request.getTitle() == null || request.getTitle().isEmpty()) { | ||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // TODO refactor to properties | ||||||||||||||||||||||||||
| String searchText = "*" + request.getTitle() + "*"; | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sanitize search input to prevent query injection The search text is directly concatenated with wildcards without escaping special characters. This could lead to query errors or injection issues if the title contains Elasticsearch query syntax characters. Consider escaping special characters: -String searchText = "*" + request.getTitle() + "*";
+String searchText = "*" + QueryStringQuery.escape(request.getTitle()) + "*";Or use a more robust approach with match queries instead of wildcards for better performance: -String searchText = "*" + request.getTitle() + "*";
-QueryStringQuery fullTextQuery = QueryStringQuery.of(builder -> builder.fields(fullTextFields).query(searchText));
+QueryStringQuery fullTextQuery = QueryStringQuery.of(builder ->
+ builder.fields(fullTextFields)
+ .query(request.getTitle())
+ .defaultOperator(Operator.And)
+ .analyzeWildcard(true)
+ .fuzziness("AUTO"));
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| List<String> fullTextFields = List.of( | ||||||||||||||||||||||||||
| "title.textValue^2", | ||||||||||||||||||||||||||
| "identifier^1" | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| QueryStringQuery fullTextQuery = QueryStringQuery.of(builder -> builder.fields(fullTextFields).query(searchText)); | ||||||||||||||||||||||||||
| query.must(fullTextQuery._toQuery()); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| protected boolean buildGroupQuery(PetriNetSearch request, LoggedUser user, Locale locale, BoolQuery.Builder query) { | ||||||||||||||||||||||||||
| if (request.getGroup() == null || request.getGroup().isEmpty()) { | ||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| PetriNetSearch processQuery = new PetriNetSearch(); | ||||||||||||||||||||||||||
| processQuery.setGroup(request.getGroup()); | ||||||||||||||||||||||||||
| List<PetriNetReference> groupProcesses = this.petriNetService.search(processQuery, user, new FullPageRequest(), locale).getContent(); | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add pagination limits to prevent memory issues. Using -List<PetriNetReference> groupProcesses = this.petriNetService.search(processQuery, user, new FullPageRequest(), locale).getContent();
+// Limit to reasonable number of group processes
+Page<PetriNetReference> groupProcessPage = this.petriNetService.search(processQuery, user, PageRequest.of(0, 1000), locale);
+if (groupProcessPage.getTotalElements() > 1000) {
+ log.warn("Large number of group processes found: {}", groupProcessPage.getTotalElements());
+}
+List<PetriNetReference> groupProcesses = groupProcessPage.getContent();📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| if (groupProcesses.isEmpty()) | ||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| BoolQuery.Builder groupQuery = new BoolQuery.Builder(); | ||||||||||||||||||||||||||
| groupProcesses.stream().map(PetriNetReference::getIdentifier) | ||||||||||||||||||||||||||
| .map(netIdentifier -> termQuery("identifier", netIdentifier)) | ||||||||||||||||||||||||||
| .forEach(termQuery -> groupQuery.should(termQuery._toQuery())); | ||||||||||||||||||||||||||
| query.filter(groupQuery.build()._toQuery()); | ||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,126 @@ | ||||||||||||||||||||||||||||||||||
| package com.netgrif.application.engine.menu.services; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.auth.service.UserService; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.elastic.service.interfaces.IElasticCaseService; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.elastic.web.requestbodies.CaseSearchRequest; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.menu.services.interfaces.DashboardItemService; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.objects.auth.domain.AbstractUser; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.objects.auth.domain.ActorTransformer; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.objects.auth.domain.LoggedUser; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.objects.petrinet.domain.throwable.TransitionNotExecutableException; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.objects.utils.MenuItemUtils; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.objects.workflow.domain.Case; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.objects.workflow.domain.Task; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.objects.workflow.domain.menu.ToDataSetOutcome; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.objects.workflow.domain.menu.dashboard.DashboardItemBody; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.objects.workflow.domain.menu.dashboard.DashboardItemConstants; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.petrinet.service.interfaces.IPetriNetService; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.startup.ImportHelper; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.workflow.service.interfaces.IDataService; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.workflow.service.interfaces.ITaskService; | ||||||||||||||||||||||||||||||||||
| import com.netgrif.application.engine.workflow.service.interfaces.IWorkflowService; | ||||||||||||||||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||||||||
| import lombok.extern.slf4j.Slf4j; | ||||||||||||||||||||||||||||||||||
| import org.springframework.data.domain.Page; | ||||||||||||||||||||||||||||||||||
| import org.springframework.data.domain.PageRequest; | ||||||||||||||||||||||||||||||||||
| import org.springframework.stereotype.Service; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import java.util.Collections; | ||||||||||||||||||||||||||||||||||
| import java.util.Locale; | ||||||||||||||||||||||||||||||||||
| import java.util.Map; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| @Slf4j | ||||||||||||||||||||||||||||||||||
| @Service | ||||||||||||||||||||||||||||||||||
| @RequiredArgsConstructor | ||||||||||||||||||||||||||||||||||
| public class DashboardItemServiceImpl implements DashboardItemService { | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| protected final IWorkflowService workflowService; | ||||||||||||||||||||||||||||||||||
| protected final UserService userService; | ||||||||||||||||||||||||||||||||||
| protected final ITaskService taskService; | ||||||||||||||||||||||||||||||||||
| protected final IDataService dataService; | ||||||||||||||||||||||||||||||||||
| protected final IPetriNetService petriNetService; | ||||||||||||||||||||||||||||||||||
| protected final IElasticCaseService elasticCaseService; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Creates a new dashboard item case if it does not already exist. | ||||||||||||||||||||||||||||||||||
| * If a case with the same ID is found, it is returned instead of creating a new one. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @param body The {@link DashboardItemBody} containing data for the dashboard item. | ||||||||||||||||||||||||||||||||||
| * @return The created or existing {@link Case} representing the dashboard item. | ||||||||||||||||||||||||||||||||||
| * @throws TransitionNotExecutableException if the task transition is not executable. | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||
| public Case getOrCreate(DashboardItemBody body) throws TransitionNotExecutableException { | ||||||||||||||||||||||||||||||||||
| MenuItemUtils.sanitize(body.getId()); | ||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate the sanitized ID before proceeding. The method sanitizes the ID but doesn't validate the result. Consider checking if the sanitized ID is valid (non-empty, follows expected format) before using it. public Case getOrCreate(DashboardItemBody body) throws TransitionNotExecutableException {
- MenuItemUtils.sanitize(body.getId());
+ String sanitizedId = MenuItemUtils.sanitize(body.getId());
+ if (sanitizedId == null || sanitizedId.trim().isEmpty()) {
+ throw new IllegalArgumentException("Invalid dashboard item ID after sanitization");
+ }
Case itemCase;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
martinkranec marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||
| Case itemCase; | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+55
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainAdd null check after sanitization The 🏁 Script executed: #!/bin/bash
# Check the implementation of MenuItemUtils.sanitize to understand its behavior
ast-grep --pattern 'sanitize($_) {
$$$
}'
# Also search for the method signature
rg -A 10 "sanitize\(" --glob "**/MenuItemUtils.java"Length of output: 1553 🏁 Script executed: #!/bin/bash
# Show context around getOrCreate in DashboardItemServiceImpl
rg -A10 -B10 "getOrCreate" --glob "**/DashboardItemServiceImpl.java"Length of output: 3247 Ensure the sanitized ID is used and validated The call to
For example: - MenuItemUtils.sanitize(body.getId());
- Case itemCase;
+ String sanitizedId = MenuItemUtils.sanitize(body.getId());
+ if (sanitizedId == null) {
+ throw new IllegalArgumentException("Dashboard item ID must not be null or empty");
+ }
+ body.setId(sanitizedId);
+ Case itemCase;
- itemCase = findById(body.getId());
+ itemCase = findById(sanitizedId);
📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| itemCase = findById(body.getId()); | ||||||||||||||||||||||||||||||||||
| if (itemCase != null) { | ||||||||||||||||||||||||||||||||||
| log.info("Dashboard item with id:{} already exists", body.getId()); | ||||||||||||||||||||||||||||||||||
| return itemCase; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| LoggedUser loggedUser = ActorTransformer.toLoggedUser(userService.getLoggedOrSystem()); | ||||||||||||||||||||||||||||||||||
| itemCase = workflowService.createCase(petriNetService.getNewestVersionByIdentifier(DashboardItemConstants.PROCESS_IDENTIFIER).getStringId(), body.getName().getDefaultValue(), "", loggedUser).getCase(); | ||||||||||||||||||||||||||||||||||
| ToDataSetOutcome outcome = body.toDataSet(); | ||||||||||||||||||||||||||||||||||
| itemCase = setDataWithExecute(itemCase, DashboardItemConstants.TASK_CONFIGURE, outcome.getDataSet()); | ||||||||||||||||||||||||||||||||||
| return itemCase; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Updates an existing dashboard item case with new data. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @param itemCase The existing {@link Case} to update. | ||||||||||||||||||||||||||||||||||
| * @param body The {@link DashboardItemBody} containing updated data. | ||||||||||||||||||||||||||||||||||
| * @return The updated {@link Case} representing the dashboard item. | ||||||||||||||||||||||||||||||||||
| * @throws TransitionNotExecutableException if the task transition is not executable. | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||
| public Case update(Case itemCase, DashboardItemBody body) throws TransitionNotExecutableException { | ||||||||||||||||||||||||||||||||||
| MenuItemUtils.sanitize(body.getId()); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+81
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add null check for itemCase parameter and validate sanitized ID. The method should validate input parameters and the sanitized ID. public Case update(Case itemCase, DashboardItemBody body) throws TransitionNotExecutableException {
+ if (itemCase == null) {
+ throw new IllegalArgumentException("itemCase cannot be null");
+ }
+ if (body == null) {
+ throw new IllegalArgumentException("body cannot be null");
+ }
- MenuItemUtils.sanitize(body.getId());
+ String sanitizedId = MenuItemUtils.sanitize(body.getId());
+ if (sanitizedId == null || sanitizedId.trim().isEmpty()) {
+ throw new IllegalArgumentException("Invalid dashboard item ID after sanitization");
+ }
ToDataSetOutcome outcome = body.toDataSet();📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| ToDataSetOutcome outcome = body.toDataSet(); | ||||||||||||||||||||||||||||||||||
| itemCase = setDataWithExecute(itemCase, DashboardItemConstants.TASK_CONFIGURE, outcome.getDataSet()); | ||||||||||||||||||||||||||||||||||
| return itemCase; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Finds an existing dashboard item case by its identifier. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @param identifier The unique identifier of the dashboard item. | ||||||||||||||||||||||||||||||||||
| * @return The {@link Case} representing the dashboard item, or null if not found. | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||
| public Case findById(String identifier) { | ||||||||||||||||||||||||||||||||||
| String query = String.format("processIdentifier:%s AND dataSet.%s.textValue.keyword:\"%s\"", | ||||||||||||||||||||||||||||||||||
| DashboardItemConstants.PROCESS_IDENTIFIER, DashboardItemConstants.FIELD_ID, identifier); | ||||||||||||||||||||||||||||||||||
| return findCase(DashboardItemConstants.PROCESS_IDENTIFIER, query); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+95
to
+98
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sanitize the identifier to prevent potential query injection. The identifier is directly inserted into the query string without sanitization, which could lead to query injection vulnerabilities if the identifier contains special characters or malicious input. public Case findById(String identifier) {
+ if (identifier == null || identifier.trim().isEmpty()) {
+ return null;
+ }
+ String sanitizedIdentifier = MenuItemUtils.sanitize(identifier);
String query = String.format("processIdentifier:%s AND dataSet.%s.textValue.keyword:\"%s\"",
- DashboardItemConstants.PROCESS_IDENTIFIER, DashboardItemConstants.FIELD_ID, identifier);
+ DashboardItemConstants.PROCESS_IDENTIFIER, DashboardItemConstants.FIELD_ID, sanitizedIdentifier);
return findCase(DashboardItemConstants.PROCESS_IDENTIFIER, query);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| protected Case setData(Case useCase, String transId, Map<String, Map<String, Object>> dataSet) { | ||||||||||||||||||||||||||||||||||
| String taskId = MenuItemUtils.findTaskIdInCase(useCase, transId); | ||||||||||||||||||||||||||||||||||
| return dataService.setData(taskId, ImportHelper.populateDataset((Map) dataSet)).getCase(); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+101
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainDocument or remove unused method. The 🏁 Script executed: #!/bin/bash
# Search for usage of setData method in the codebase
rg -A 2 "setData\(" --type java | grep -B 2 -A 2 "DashboardItemServiceImpl"Length of output: 1756 🏁 Script executed: #!/bin/bash
# Show the beginning of MenuItemService to identify its type and declaration of setData
sed -n '1,200p' application-engine/src/main/java/com/netgrif/application/engine/menu/services/MenuItemService.java
echo '---'
# Show the beginning of DashboardItemServiceImpl to see its class signature and overrides
sed -n '1,200p' application-engine/src/main/java/com/netgrif/application/engine/menu/services/DashboardItemServiceImpl.javaLength of output: 15957 Remove or document unused The protected 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| @SuppressWarnings({"unchecked", "rawtypes"}) | ||||||||||||||||||||||||||||||||||
| protected Case setDataWithExecute(Case useCase, String transId, Map<String, Map<String, Object>> dataSet) throws TransitionNotExecutableException { | ||||||||||||||||||||||||||||||||||
| AbstractUser loggedUser = userService.getLoggedOrSystem(); | ||||||||||||||||||||||||||||||||||
| String taskId = MenuItemUtils.findTaskIdInCase(useCase, transId); | ||||||||||||||||||||||||||||||||||
| Task task = taskService.findOne(taskId); | ||||||||||||||||||||||||||||||||||
| task = taskService.assignTask(task, loggedUser).getTask(); | ||||||||||||||||||||||||||||||||||
| task = dataService.setData(task, ImportHelper.populateDataset((Map) dataSet)).getTask(); | ||||||||||||||||||||||||||||||||||
| return taskService.finishTask(task, loggedUser).getCase(); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| protected Case findCase(String processIdentifier, String query) { | ||||||||||||||||||||||||||||||||||
| CaseSearchRequest request = CaseSearchRequest.builder() | ||||||||||||||||||||||||||||||||||
| .process(Collections.singletonList(new CaseSearchRequest.PetriNet(processIdentifier))) | ||||||||||||||||||||||||||||||||||
| .query(query) | ||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||
| Page<Case> resultPage = elasticCaseService.search(java.util.List.of(request), ActorTransformer.toLoggedUser(userService.getLoggedOrSystem()), | ||||||||||||||||||||||||||||||||||
| PageRequest.of(0, 1), Locale.getDefault(), false); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return resultPage.hasContent() ? resultPage.getContent().get(0) : null; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Credentials echoed to logs – remove immediately
echo "$MAVEN_USERNAME / $MAVEN_PASSWORD"leaks raw credentials into the Actions log. Even if GitHub masks secrets, any mis-configuration or later copy-paste can expose them.Delete this debug step or output only a placeholder.
📝 Committable suggestion
🤖 Prompt for AI Agents