Skip to content
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

[JENKINS-55927] Hook event should not trigger Branch Indexing #908

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@
* Abstract hook processor.
*
* Add new hook processors by extending this class and implement {@link #process(HookEventType, String, BitbucketType, String)},
* extract owner and repository name from the hook payload and then call {@link #scmSourceReIndex(String, String)}
* to launch a branch/PR reindexing on the matching SCMSource.
* extract details from the hook payload and then fire an {@link jenkins.scm.api.SCMEvent} to dispatch it to the SCM API.
*/
public abstract class HookProcessor {

Expand Down Expand Up @@ -86,10 +85,11 @@
/**
* To be called by implementations once the owner and the repository have been extracted from the payload.
*
* @deprecated Branch Indexing should not be triggered directly. But through {@link jenkins.scm.api.SCMSourceEvent}.
* @param owner the repository owner as configured in the SCMSource
* @param repository the repository name as configured in the SCMSource
*/
protected void scmSourceReIndex(final String owner, final String repository) {

Check warning on line 92 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/hooks/HookProcessor.java

View check run for this annotation

ci.jenkins.io / Java Compiler

compiler:compile

NORMAL: deprecated item is not annotated with @deprecated
try (ACLContext context = ACL.as(ACL.SYSTEM)) {
boolean reindexed = false;
for (SCMSourceOwner scmOwner : SCMSourceOwners.all()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@

public class NativeServerPushHookProcessor extends HookProcessor {

private static final boolean SCAN_ON_PUSH_WITH_EMPTY_CHANGES = Boolean.getBoolean(
NativeServerPushHookProcessor.class.getName()+".scanOnPushWithEmptyChanges");

private static final Logger LOGGER = Logger.getLogger(NativeServerPushHookProcessor.class.getName());

@Override
Expand Down Expand Up @@ -102,10 +105,10 @@ public void process(HookEventType hookEvent, String payload, BitbucketType insta
return;
}

if (changes.isEmpty()) {
if (SCAN_ON_PUSH_WITH_EMPTY_CHANGES && changes.isEmpty()) {
final String owner = repository.getOwnerName();
final String repositoryName = repository.getRepositoryName();
LOGGER.log(Level.INFO, "Received hook from Bitbucket. Processing push event on {0}/{1}",
LOGGER.log(Level.INFO, "Received push hook with empty changes from Bitbucket. Processing push event on {0}/{1}",
new Object[] { owner, repositoryName });
scmSourceReIndex(owner, repositoryName);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@

public class PushHookProcessor extends HookProcessor {

private static final boolean SCAN_ON_PUSH_WITH_EMPTY_CHANGES = Boolean.getBoolean(
PushHookProcessor.class.getName()+".scanOnPushWithEmptyChanges");

private static final Logger LOGGER = Logger.getLogger(PushHookProcessor.class.getName());

@Override
Expand All @@ -71,15 +74,15 @@
push = BitbucketCloudWebhookPayload.pushEventFromPayload(payload);
}
if (push != null) {
String owner = push.getRepository().getOwnerName();
Copy link
Member

@nfalco79 nfalco79 Nov 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As described in JIRA issue I think these changes will cause to the user that have setup "Discover pull requests" strategies different than "The current pull request revision" they never get a build for PRs which target branch is the same branch subject of the merged.
If I understand correct the configuration in the JIRA issue the reported would trigger manual reindex to update all PRs. Than this should be archived using a build strategies to ignore some specific index events (maybe already implemented in other plugins)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JIRA mentions the Received hook from Bitbucket. Processing push event on log line. per my reading, this is really coming from the event subscriber / hook processors (entry point is BitbucketSCMSourcePushHookReceiver). It would happen regardless of the multibranch configuration as far as I can tell.

final String repository = push.getRepository().getRepositoryName();
if (push.getChanges().isEmpty()) {
LOGGER.log(Level.INFO, "Received hook from Bitbucket. Processing push event on {0}/{1}",
new Object[]{owner, repository});
if (SCAN_ON_PUSH_WITH_EMPTY_CHANGES && push.getChanges().isEmpty()) {
final String owner = push.getRepository().getOwnerName();
final String repository = push.getRepository().getRepositoryName();
LOGGER.log(Level.INFO, "Received push hook with empty changes from Bitbucket. Processing push event on {0}/{1}",
new Object[]{owner, repository});
Fixed Show fixed Hide fixed
scmSourceReIndex(owner, repository);
} else {
SCMHeadEvent.Type type = null;
for (BitbucketPushEvent.Change change: push.getChanges()) {
for (BitbucketPushEvent.Change change : push.getChanges()) {
if ((type == null || type == SCMEvent.Type.CREATED) && change.isCreated()) {
type = SCMEvent.Type.CREATED;
} else if ((type == null || type == SCMEvent.Type.REMOVED) && change.isClosed()) {
Expand Down Expand Up @@ -170,7 +173,7 @@
}

Map<SCMHead, SCMRevision> result = new HashMap<>();
for (BitbucketPushEvent.Change change: getPayload().getChanges()) {
for (BitbucketPushEvent.Change change : getPayload().getChanges()) {
if (change.isClosed()) {
result.put(new BranchSCMHead(change.getOld().getName()), null);
} else {
Expand Down Expand Up @@ -199,7 +202,7 @@

@Override
public boolean isMatch(@NonNull SCM scm) {
// TODO

Check warning on line 205 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/hooks/PushHookProcessor.java

View check run for this annotation

ci.jenkins.io / Open Tasks Scanner

TODO

NORMAL:
return false;
}
}, BitbucketSCMSource.getEventDelaySeconds(), TimeUnit.SECONDS);
Expand Down
Loading