You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "mirsilstan (via GitHub)" <gi...@apache.org> on 2025/04/07 08:05:01 UTC
[PR] SLING-12737 The method findJobs in JobManagerImpl does not accept a template containing an operation [sling-org-apache-sling-event]
mirsilstan opened a new pull request, #40:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/40
The property name is now encoded only after extracting the operation from the first 1 or 2 character of the key.
I added a test in HistoryIT that retrieves the jobs between a start date and an end date.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12737 The method findJobs in JobManagerImpl does not accept a template containing an operation [sling-org-apache-sling-event]
Posted by "mirsilstan (via GitHub)" <gi...@apache.org>.
mirsilstan commented on PR #40:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/40#issuecomment-2785561910
> @mirsilstan thanks for your contribution.
>
> If I understand your description in [SLING-12737](https://issues.apache.org/jira/browse/SLING-12737), the code in `JobSchedulerImpl` is correct and would be able to handle the templates as intended, but not in the `JobManagerImpl`; is my understanding correct?
>
> I would appreciate a bit of test coverage for the code you extracted into the `PropertyNameAndOperationExtractor`class.
The only reason I added the extra classes is because the code in JobManagerImpl and the code in JobSchedulerImpl is duplicated.
I thought about creating a single method and reusing it in both places.
However, this increases the number of changes.
Hence I removed the extra classes and I changed only 2 lines which are strictly related to the bug.
The code is tested in HistoryIT, if you run the test on the main branch, it will fail.
Please advise if any further tests are necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12737 The method findJobs in JobManagerImpl does not accept a template containing an operation [sling-org-apache-sling-event]
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on PR #40:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/40#issuecomment-2783971110
@mirsilstan thanks for your contribution.
If I understand your description in SLING-12737, the code in ``JobSchedulerImpl`` is correct and would be able to handle the templates as intended, but not in the ``JobManagerImpl``; is my understanding correct?
I would appreciate a bit of test coverage for the code you extracted into the ``PropertyNameAndOperationExtractor``class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12737 The method findJobs in JobManagerImpl does not accept a template containing an operation [sling-org-apache-sling-event]
Posted by "mirsilstan (via GitHub)" <gi...@apache.org>.
mirsilstan commented on code in PR #40:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/40#discussion_r2032610134
##########
src/main/java/org/apache/sling/event/impl/support/PropertyNameAndOperationExtractor.java:
##########
@@ -0,0 +1,37 @@
+package org.apache.sling.event.impl.support;
+
+public class PropertyNameAndOperationExtractor {
+ public PropertyNameAndOperation extractPropertyNameAndOperation(String key) {
Review Comment:
I removed all extra classes, please see comment below.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12737 The method findJobs in JobManagerImpl does not accept a template containing an operation [sling-org-apache-sling-event]
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #40:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/40#discussion_r2031610401
##########
src/main/java/org/apache/sling/event/impl/support/PropertyNameAndOperationExtractor.java:
##########
@@ -0,0 +1,37 @@
+package org.apache.sling.event.impl.support;
+
+public class PropertyNameAndOperationExtractor {
+ public PropertyNameAndOperation extractPropertyNameAndOperation(String key) {
Review Comment:
I would convert this into a static method, then you won't need to invoke the constructor anymore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12737 The method findJobs in JobManagerImpl does not accept a template containing an operation [sling-org-apache-sling-event]
Posted by "stefan-egli (via GitHub)" <gi...@apache.org>.
stefan-egli merged PR #40:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/40
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org