Skip to content

Rule 606 Avoid HTTP calls inside loops - #455

Open
TchaloSon wants to merge 1 commit into
green-code-initiative:mainfrom
TchaloSon:rule/GCI606
Open

Rule 606 Avoid HTTP calls inside loops#455
TchaloSon wants to merge 1 commit into
green-code-initiative:mainfrom
TchaloSon:rule/GCI606

Conversation

@TchaloSon

Copy link
Copy Markdown

Avoid HTTP calls inside loops

Groupe shrek les dépendances moisies(CGI)

@rducasse rducasse left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree with the rule in principle — HTTP calls in loops is a genuine N+1 pattern with real environmental impact, and it maps well to the existing GCI1 rule (Spring repository calls in loops).

A few points before we can approve:

Blocking:

  • Implementation scope is unclear. The asciidoc only shows RestTemplate, but an HTTP call can go through dozens of libraries (WebClient, HttpClient (JDK 11+), OkHttpClient, Apache HttpClient, Feign, generated OpenAPI clients, custom wrappers…). Please clarify in the asciidoc exactly which libraries/methods are in scope for detection. Without this, the Java plugin implementer cannot write a deterministic check.
  • The exception "loop limited to a small number of iterations" is not implementable at static analysis time — the compiler cannot know how many elements a list will contain at runtime. Either remove this exception or rephrase it as something statically detectable (e.g., a loop over a literal array of ≤ N elements).
  • Environmental gains are not sourced. Please add a == References section with at least one external source documenting the impact of N+1 HTTP patterns (latency studies, bandwidth overhead measurements, etc.).

Important:

  • The compliant solution assumes a batch API exists on the remote service, which is often not the case for third-party APIs. Consider adding an alternative compliant pattern (e.g., caching, moving the call outside the loop when possible) for cases where no batch endpoint is available.

Minor:

  • The Exceptions section says "when the HTTP call is required by an external API that does not provide a batch endpoint" — if this exception is kept, the rule would need a way for developers to suppress it. Consider documenting the use of @SuppressWarnings or a SonarQube // NOSONAR annotation.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a new Creedengo rule specification (GCI606) documenting why HTTP calls inside loops are inefficient and how to refactor toward batching.

Changes:

  • Introduces the Java rule documentation for GCI606 with non-compliant and compliant examples.
  • Adds the GCI606 rule metadata JSON (title, severity, remediation, tags).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/main/rules/GCI606/java/GCI606.asciidoc Documents the rule rationale, examples, and exceptions for Java.
src/main/rules/GCI606/GCI606.json Declares rule metadata (severity, remediation, tags).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

"https://api.example.com/users/batch",
userIds,
UserDetails[].class
);

== Exceptions

This rule should not report an issue when the loop is explicitly limited to a very small number of iterations,or when the HTTP call is required by an external API that does not provide a batch endpoint. No newline at end of file
@github-actions

Copy link
Copy Markdown
Contributor

This PR has been automatically marked as stale because it has no activity for 60 days.
Please add a comment if you want to keep the issue open. Thank you for your contributions!

@github-actions github-actions Bot added the stale label Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

5 participants