Story/cite 132 1 #267

sathish2379 · 2024-05-03T19:57:08Z

Guidelines for Pull Requests

If you haven't yet read our code review guidelines, please do so, You can find them here.

Please confirm the following by adding an x for each item (turn [ ] into [x]).

I have removed all code style changes that are not necessary (e.g. changing blanks across the whole file that don’t need to be changed, adding empty lines in parts other than your own code)
I am not making any changes to files that don’t have any effect (e.g. imports added that don’t need to be added)
I do not have any sysout statements in my code or commented out code that isn’t needed anymore
I am not reformatting any files in the wrong format or without cause.
I am not changing file encoding or line endings to something else than UTF-8, LF
My pull request does not show an insane amount of files being changed although my ticket only requires a few files being changed
I have added Javadoc/documentation where appropriate
I have added test cases where appropriate
I have explained any part of my code/implementation decisions that is not be self-explanatory

Please provide a brief description of your ticket

... Put ticket description here and add link to ticket ...
Right now, a job is just marked as cancelled (for jobs that silently failed) but not aborted. The sync processor should check in regular intervals if the current job is still in progress or if it should be cancelled.
https://diging.atlassian.net/browse/CITE-132

Are there any other pull requests that this one depends on?

Here is the previous PR: #212

Anything else the reviewer needs to know?

... describe here ...

[CITE-132] merging develop to update the branch to latest

…ncJobManager

diging-jenkins · 2024-05-03T19:57:11Z

Can one of the admins verify this patch?

jdamerow · 2024-05-14T16:27:30Z

...sphere/src/main/java/edu/asu/diging/citesphere/core/service/impl/AsyncCitationProcessor.java

        jobRepo.save(job);
+
+        Future<String> result = new AsyncResult<String>(job.getId());
+        return result;


merge with line 170

jdamerow · 2024-05-14T16:39:15Z

citesphere/src/main/java/edu/asu/diging/citesphere/core/service/impl/CitationManager.java

+        if (existingFuture != null && !existingFuture.isDone()) {
+            existingFuture.cancel(true);
+            futureMap.remove(groupId);
+        }


why is the job cancelled here?

jdamerow · 2024-05-14T16:42:21Z

...sphere/src/main/java/edu/asu/diging/citesphere/core/service/impl/AsyncCitationProcessor.java

+        return result;
+    }
+
+    private boolean checkIfThreadIsInterrupted(Thread thread, GroupSyncJob job, String groupId) {


this does not only check but also set the task to cancelled

jdamerow · 2024-05-14T16:43:02Z

...sphere/src/main/java/edu/asu/diging/citesphere/core/service/impl/AsyncCitationProcessor.java

        job.setStatus(JobStatus.PREPARED);
        jobRepo.save(job);
        jobManager.addJob(job);
+        Thread currentThread = Thread.currentThread();


I think you can just do this in checkIfThreadIsInterrupted and remove the first parameter of the method.

jdamerow · 2024-05-15T14:49:44Z

...sphere/src/main/java/edu/asu/diging/citesphere/core/service/impl/AsyncCitationProcessor.java

+
        syncCitations(user, groupId, job, versions, counter);
+
+        if(checkIfThreadIsInterruptedAndCancelJob(job, groupId)) {


Actually, now that I'm looking at this again, you will want to cancel the thread within the syncCitations and probably also syncCollections method. They are the long running methods that need to be cancelled if cancellation was requested. Otherwise, if there are 1000 citations to sync it would still go through all of them before cancelling.

jdamerow · 2024-05-15T14:49:58Z

...sphere/src/main/java/edu/asu/diging/citesphere/core/service/impl/AsyncCitationProcessor.java

+
+    private boolean checkIfThreadIsInterruptedAndCancelJob(GroupSyncJob job, String groupId) {
+    	Thread thread = Thread.currentThread();
+        if(thread.isInterrupted()) {


merge with 173

jdamerow · 2024-05-15T14:50:27Z

citesphere/src/main/java/edu/asu/diging/citesphere/core/service/jobs/impl/SyncJobManager.java

    @Override
    public void cancelJob(String jobId) {
        Optional<GroupSyncJob> jobOptional = jobRepo.findById(jobId);
+        String groupId;


declare where needed

jdamerow · 2024-05-23T17:29:45Z

Make it so, Jenkins.

diging-jenkins · 2024-05-23T17:30:39Z

Jenkins successfully deployed Citesphere to be reviewed!

jdamerow · 2024-05-23T17:50:38Z

Not sure why, but the items table for a group is now empty.

diging-jenkins · 2025-09-26T16:02:30Z

Jenkins successfully deployed Citesphere to be reviewed!

jdamerow · 2025-09-26T16:04:07Z

java.lang.NullPointerException
	edu.asu.diging.citesphere.core.service.jobs.impl.SyncJobManager.cancelJob(SyncJobManager.java:83)
	edu.asu.diging.citesphere.web.user.CancelSyncJobController.cancel(CancelSyncJobController.java:19)
	java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	java.base/java.lang.reflect.Method.invoke(Method.java:566)
	org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:205)
	org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:150)
	org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:117)

diging-jenkins · 2025-09-26T19:18:23Z

Jenkins successfully deployed Citesphere to be reviewed!

Charishma0249 and others added 9 commits May 17, 2022 11:43

[CITE-132] Addressing review comments-Implementing future cancel changes

80c7f99

[CITE-132]Addressing review comments-implementing java future cancel job

3a42c7b

[CITE-132] Addressed review comments - code cleanup

65d89c3

Merge branch 'develop' into story/CITE-132-1

53fefbb

[CITE-132] merging develop to update the branch to latest

[CITE-132] update cancel method return and updated cancel check in Sy…

2f0a2fc

…ncJobManager

[CITE-132] Using concurrent Hashmap for storing future

3d493f8

[CITE 132] Removed Sysout statements

2e5c969

[CITE 132] removed extra spaces, tabs

10d9352

[CITE 132] spaces fixes

03e5ec3

[CITE 132] spaces fixes

d36fdad

sathish2379 requested a review from jdamerow May 3, 2024 20:03

jdamerow requested changes May 14, 2024

View reviewed changes

jdamerow closed this May 14, 2024

sathish2379 added 2 commits May 14, 2024 12:42

[CITE-132] refactor CheckIfInterrupted method

b80eb69

[CITE-132] cancelling of job is not required in getGroupItems

1b033b9

sathish2379 reopened this May 14, 2024

jdamerow requested changes May 15, 2024

View reviewed changes

jdamerow closed this May 15, 2024

[CITE-132] cancelling job within sync

55723b7

sathish2379 reopened this May 22, 2024

[CITE-132] correcting indentations

706db11

jdamerow closed this May 23, 2024

rajvi-patel-22 added 2 commits July 25, 2025 18:11

[CITE-132] Handle race condition for sync

c421f37

[CITE-132] Comment out code to check race condition

1ff5c11

jdamerow reopened this Sep 26, 2025

jdamerow closed this Sep 26, 2025

Merge branch 'develop' into story/CITE-132-1

a64b7d3

rajvi-patel-22 reopened this Sep 26, 2025


		syncCitations(user, groupId, job, versions, counter);

		if(checkIfThreadIsInterruptedAndCancelJob(job, groupId)) {

Story/cite 132 1 #267

Are you sure you want to change the base?

Story/cite 132 1 #267

Uh oh!

Conversation

sathish2379 commented May 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Guidelines for Pull Requests

Please provide a brief description of your ticket

Are there any other pull requests that this one depends on?

Anything else the reviewer needs to know?

Uh oh!

diging-jenkins commented May 3, 2024

Uh oh!

jdamerow May 14, 2024

Choose a reason for hiding this comment

Uh oh!

jdamerow May 14, 2024

Choose a reason for hiding this comment

Uh oh!

jdamerow May 14, 2024

Choose a reason for hiding this comment

Uh oh!

jdamerow May 14, 2024

Choose a reason for hiding this comment

Uh oh!

jdamerow May 15, 2024

Choose a reason for hiding this comment

Uh oh!

jdamerow May 15, 2024

Choose a reason for hiding this comment

Uh oh!

jdamerow May 15, 2024

Choose a reason for hiding this comment

Uh oh!

jdamerow commented May 23, 2024

Uh oh!

diging-jenkins commented May 23, 2024

Uh oh!

jdamerow commented May 23, 2024

Uh oh!

diging-jenkins commented Sep 26, 2025

Uh oh!

jdamerow commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

diging-jenkins commented Sep 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sathish2379 commented May 3, 2024 •

edited

Loading

jdamerow commented Sep 26, 2025 •

edited

Loading