diff --git a/.github/workflows/verify-api-backward-compatibility.yml b/.github/workflows/verify-api-backward-compatibility.yml new file mode 100644 index 000000000..d154cb58b --- /dev/null +++ b/.github/workflows/verify-api-backward-compatibility.yml @@ -0,0 +1,225 @@ +name: Verify API Backward Compatibility + +on: [pull_request] + +permissions: + contents: read + pull-requests: write + +jobs: + api-compatibility-check: + runs-on: ubuntu-24.04 + timeout-minutes: 15 + + env: + TZ: Asia/Kolkata + + steps: + - name: Checkout base branch + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v5 + with: + repository: ${{ github.event.pull_request.base.repo.full_name }} + ref: ${{ github.event.pull_request.base.ref }} + fetch-depth: 0 + path: baseline + + - name: Set up JDK 21 + uses: actions/setup-java@v5 + with: + distribution: 'zulu' + java-version: '21' + + - name: Generate baseline spec + working-directory: baseline + run: ./gradlew :fineract-provider:resolve --no-daemon + + - name: Checkout PR branch + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v5 + with: + repository: ${{ github.event.pull_request.head.repo.full_name }} + ref: ${{ github.event.pull_request.head.sha }} + fetch-depth: 0 + path: current + + - name: Generate PR spec + working-directory: current + run: ./gradlew :fineract-provider:resolve --no-daemon + + - name: Sanitize specs + run: | + python3 -c " + import json, sys + + def sanitize(path): + with open(path) as f: + spec = json.load(f) + fixed = 0 + for path_item in spec.get('paths', {}).values(): + for op in path_item.values(): + if not isinstance(op, dict) or 'requestBody' not in op: + continue + for media in op['requestBody'].get('content', {}).values(): + if 'schema' not in media: + media['schema'] = {'type': 'object'} + fixed += 1 + if fixed: + with open(path, 'w') as f: + json.dump(spec, f) + print(f'{path}: fixed {fixed} entries') + + sanitize('${GITHUB_WORKSPACE}/baseline/fineract-provider/build/resources/main/static/fineract.json') + sanitize('${GITHUB_WORKSPACE}/current/fineract-provider/build/resources/main/static/fineract.json') + " + + - name: Check breaking changes + id: breaking-check + continue-on-error: true + working-directory: current + run: | + set -o pipefail + ./gradlew :fineract-provider:checkBreakingChanges \ + -PapiBaseline="${GITHUB_WORKSPACE}/baseline/fineract-provider/build/resources/main/static/fineract.json" \ + --no-daemon --quiet 2>&1 | tail -50 + + - name: Build report + if: steps.breaking-check.outcome == 'failure' + id: report + run: | + REPORT_DIR="current/fineract-provider/build/swagger-brake" + + python3 -c " + import json, glob, os + from collections import defaultdict + + RULE_DESC = { + 'R001': 'Standard API changed to beta', + 'R002': 'Path deleted', + 'R003': 'Request media type deleted', + 'R004': 'Request parameter deleted', + 'R005': 'Request parameter enum value deleted', + 'R006': 'Request parameter location changed', + 'R007': 'Request parameter made required', + 'R008': 'Request parameter type changed', + 'R009': 'Request attribute removed', + 'R010': 'Request type changed', + 'R011': 'Request enum value deleted', + 'R012': 'Response code deleted', + 'R013': 'Response media type deleted', + 'R014': 'Response attribute removed', + 'R015': 'Response type changed', + 'R016': 'Response enum value deleted', + 'R017': 'Request parameter constraint changed', + } + + report_dir = '${REPORT_DIR}' + files = sorted(glob.glob(os.path.join(report_dir, '*.json'))) + if not files: + body = 'Breaking change detected but no report file found.' + else: + with open(files[0]) as f: + data = json.load(f) + + all_changes = [] + for items in data.get('breakingChanges', {}).values(): + all_changes.extend(items) + + if not all_changes: + body = 'Breaking change detected but no details available in report.' + else: + def detail(c): + for key in ('attributeName', 'attribute', 'name', 'mediaType', 'enumValue', 'code'): + v = c.get(key) + if v: + val = v.rsplit('.', 1)[-1] + if key in ('attributeName', 'attribute', 'name'): + return val + return f'{key}={val}' + return '-' + + groups = defaultdict(list) + for c in all_changes: + groups[(c.get('ruleCode', '?'), detail(c))].append(c) + + lines = [] + lines.append('| Rule | Description | Detail | Affected endpoints | Count |') + lines.append('|------|-------------|--------|--------------------|-------|') + for (rule, det), items in sorted(groups.items()): + desc = RULE_DESC.get(rule, '') + eps = sorted(set( + f'{c.get(\"method\",\"\")} {c.get(\"path\",\"\")}' + for c in items if c.get('path') + )) + ep_str = ', '.join(f'\`{e}\`' for e in eps[:5]) + if len(eps) > 5: + ep_str += f' +{len(eps)-5} more' + lines.append(f'| {rule} | {desc} | \`{det}\` | {ep_str} | {len(items)} |') + + lines.append('') + lines.append(f'**Total: {len(all_changes)} violations across {len(groups)} unique changes**') + body = '\n'.join(lines) + + with open(os.environ['GITHUB_OUTPUT'], 'a') as f: + f.write('has_report=true\n') + + report_file = '${GITHUB_WORKSPACE}/breaking-changes-report.md' + with open(report_file, 'w') as f: + f.write('## Breaking API Changes Detected\n\n') + f.write(body) + f.write('\n\n> **Note:** This check is informational only and does not block the PR.\n') + + # Also write to step summary + with open(os.environ['GITHUB_STEP_SUMMARY'], 'a') as f: + f.write('## Breaking API Changes Detected\n\n') + f.write(body) + f.write('\n\n> **Note:** This check is informational only and does not block the PR.\n') + " + + - name: Comment on PR + if: always() + continue-on-error: true + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + MARKER="" + + # Find existing comment by marker + COMMENT_ID=$(gh api "repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" \ + --jq ".[] | select(.body | contains(\"${MARKER}\")) | .id" | head -1) + + if [ "${{ steps.breaking-check.outcome }}" == "failure" ] && [ -f "${GITHUB_WORKSPACE}/breaking-changes-report.md" ]; then + # Prepend marker to the report + BODY="${MARKER} + $(cat ${GITHUB_WORKSPACE}/breaking-changes-report.md)" + + if [ -n "$COMMENT_ID" ]; then + gh api "repos/${{ github.repository }}/issues/comments/${COMMENT_ID}" \ + -X PATCH -f body="${BODY}" + else + gh pr comment "${PR_NUMBER}" --repo ${{ github.repository }} --body "${BODY}" + fi + elif [ -n "$COMMENT_ID" ]; then + # No breaking changes anymore, delete the old comment + gh api "repos/${{ github.repository }}/issues/comments/${COMMENT_ID}" -X DELETE + fi + + - name: Report no breaking changes + if: steps.breaking-check.outcome == 'success' + run: | + echo "## No Breaking API Changes Detected" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "The API contract is backward compatible." >> $GITHUB_STEP_SUMMARY + + - name: Archive breaking change report + if: always() + uses: actions/upload-artifact@v4 + with: + name: api-compatibility-report + path: current/fineract-provider/build/swagger-brake/ + retention-days: 30 + + - name: Fail if breaking changes detected + if: steps.breaking-check.outcome == 'failure' + run: | + echo "::error::Breaking API changes detected. See the report above for details." + exit 1 diff --git a/build.gradle b/build.gradle index 6df313b1a..bc1361269 100644 --- a/build.gradle +++ b/build.gradle @@ -125,6 +125,7 @@ plugins { id 'com.gradleup.shadow' version '8.3.5' apply false id 'me.champeau.jmh' version '0.7.1' apply false id 'org.cyclonedx.bom' version '3.1.0' apply false + id 'com.docktape.swagger-brake' version '2.7.0' apply false } apply from: "${rootDir}/buildSrc/src/main/groovy/org.apache.fineract.release.gradle" diff --git a/fineract-doc/src/docs/en/chapters/architecture/api-backward-compatibility.adoc b/fineract-doc/src/docs/en/chapters/architecture/api-backward-compatibility.adoc new file mode 100644 index 000000000..59eb81c4f --- /dev/null +++ b/fineract-doc/src/docs/en/chapters/architecture/api-backward-compatibility.adoc @@ -0,0 +1,190 @@ += API Backward Compatibility + +== Overview + +Apache Fineract enforces API backward compatibility using https://github.com/docktape/swagger-brake[swagger-brake], an automated tool that compares OpenAPI specifications between the base branch and a pull request to detect breaking changes. This ensures that existing API consumers are not broken when new changes are deployed. + +The check runs automatically on every pull request via the `verify-api-backward-compatibility.yml` GitHub Actions workflow. + +== How It Works + +The workflow follows these steps: + +. **Generate baseline spec** — Checks out the base branch (e.g. `develop`) and runs `./gradlew :fineract-provider:resolve` to generate the current OpenAPI specification. +. **Generate PR spec** — Checks out the PR branch and generates its OpenAPI specification. +. **Sanitize specs** — Patches known issues in the generated specs (e.g. missing `schema` entries in `requestBody` content) to prevent false positives. +. **Compare** — Runs `checkBreakingChanges` via the swagger-brake Gradle plugin to compare old vs new specs. +. **Report** — If breaking changes are found: +** A deduplicated summary table is written to the GitHub Actions Step Summary (visible on the workflow run page). +** A comment is posted on the PR (when token permissions allow). +** The full JSON report is archived as a build artifact. +** The workflow **fails**, blocking the PR. + +== Breaking Change Rules + +swagger-brake detects the following categories of breaking changes: + +=== Endpoint Rules + +[cols="1,3", options="header"] +|=== +| Rule | Description +| R001 | A stable (non-beta) API was changed to beta +| R002 | An API path was deleted +|=== + +=== Request Rules + +[cols="1,3", options="header"] +|=== +| Rule | Description +| R003 | A request media type (content type) was removed +| R004 | A request parameter was deleted +| R005 | An enum value was removed from a request parameter +| R006 | A parameter location changed (e.g. `query` to `header`) +| R007 | A parameter was made required +| R008 | A parameter type was changed +| R009 | An attribute was removed from a request body schema +| R010 | A property type was changed in a request schema +| R011 | An enum value was removed from a request body schema +|=== + +=== Response Rules + +[cols="1,3", options="header"] +|=== +| Rule | Description +| R012 | A response code was deleted +| R013 | A response media type was removed +| R014 | An attribute was removed from a response schema +| R015 | A property type was changed in a response schema +| R016 | An enum value was removed from a response schema +|=== + +=== Constraint Rules + +[cols="1,3", options="header"] +|=== +| Rule | Description +| R017 | A request parameter constraint was tightened (covers `maxLength`, `minLength`, `maximum`, `minimum`, `maxItems`, `minItems`, `uniqueItems`) +|=== + +== Gradle Configuration + +The swagger-brake plugin is configured in `fineract-provider/build.gradle`: + +[source,groovy] +---- +apply plugin: 'com.docktape.swagger-brake' + +swaggerBrake { + newApi = "${project.buildDir}/resources/main/static/fineract.json" + oldApi = findProperty('apiBaseline') ?: "${projectDir}/config/swagger/fineract-baseline.json" + outputFormats = ['JSON'] + outputFilePath = "${project.buildDir}/swagger-brake" + deprecatedApiDeletionAllowed = true + strictValidation = false +} +---- + +=== Configuration Options + +[cols="2,1,4", options="header"] +|=== +| Option | Default | Description +| `newApi` | — | Path to the new (PR branch) OpenAPI spec. Generated by the `resolve` task. +| `oldApi` | — | Path to the baseline OpenAPI spec. Provided via `-PapiBaseline` in CI, or falls back to a local file. +| `outputFormats` | `['STDOUT', 'HTML']` | Report formats. We use `['JSON']` to avoid STDOUT spam and parse the report programmatically. +| `outputFilePath` | `build/swagger-brake` | Directory for generated reports. +| `deprecatedApiDeletionAllowed` | `true` | When `true`, removing a deprecated endpoint is NOT a breaking change. +| `strictValidation` | `true` | When `false`, schemas without an explicit `type` field log a warning instead of failing. Set to `false` for Fineract because the generated spec has many type-less schemas. +| `excludedPaths` | `[]` | List of path prefixes to skip (e.g. `['/v1/smscampaigns', '/v1/internal']`). Useful for excluding endpoints undergoing cleanup. +| `ignoredBreakingChangeRules` | `[]` | List of rule codes to suppress entirely (e.g. `['R001']`). +| `betaApiExtensionName` | `x-beta-api` | Vendor extension name for marking beta APIs. Beta endpoints can be freely modified without triggering violations. +| `maxLogSerializationDepth` | `3` | Controls nested object serialization depth in logs (range 1-20). Increase if you see `StackOverflowError` from circular schema references. +|=== + +== Running Locally + +To run the check locally, you need a baseline spec to compare against: + +[source,bash] +---- +# 1. Generate the baseline from develop +git stash +git checkout develop +./gradlew :fineract-provider:resolve --no-daemon +cp fineract-provider/build/resources/main/static/fineract.json /tmp/baseline.json +git checkout - +git stash pop + +# 2. Generate your current spec and compare +./gradlew :fineract-provider:checkBreakingChanges \ + -PapiBaseline="/tmp/baseline.json" \ + --no-daemon +---- + +The JSON report is written to `fineract-provider/build/swagger-brake/`. + +== Handling Breaking Changes + +=== Intentional Breaking Changes + +If your PR intentionally introduces a breaking API change (e.g. removing a deprecated field): + +. The workflow will fail and report the violations. +. Document the breaking change in the PR description with justification. +. A committer will review and approve the PR with the understanding that the API contract is changing. + +=== Excluding Paths Under Cleanup + +If you need to fix incorrect API annotations on endpoints that are not yet stable, use `excludedPaths` to temporarily exclude them from checking: + +[source,groovy] +---- +swaggerBrake { + excludedPaths = [ + '/v1/smscampaigns', + '/v1/email', + ] +} +---- + +Path exclusion is **prefix-based** — excluding `/v1/smscampaigns` will skip all paths starting with that prefix. + +Remove the exclusion once the cleanup is complete. + +=== Marking Endpoints as Beta + +For endpoints that are experimental or under active development, mark them as beta in the Java code: + +[source,java] +---- +@Operation( + summary = "...", + extensions = @Extension( + properties = @ExtensionProperty(name = "x-beta-api", value = "true") + ) +) +---- + +Beta endpoints can be freely modified, created, or removed without triggering violations. Promoting a beta endpoint to stable (removing the extension) is also non-breaking. However, demoting a stable endpoint to beta **is** a breaking change (R001). + +== Report Format + +When breaking changes are detected, the workflow produces a deduplicated summary table: + +[cols="1,2,1,3,1", options="header"] +|=== +| Rule | Description | Detail | Affected endpoints | Count +| R014 | Response attribute removed | `totalOverpaid` | `GET /v1/loans`, `GET /v1/loans/{loanId}`, `GET /v1/loans/external-id/{loanExternalId}` | 3 +|=== + +The deduplication groups violations by rule code and affected attribute, collapsing multiple endpoint occurrences into a single row. This is important because a single schema change (e.g. removing a field from a shared response type) can generate dozens of raw violations — one per endpoint that uses that schema. + +== Tool Reference + +* **Tool**: https://github.com/docktape/swagger-brake[swagger-brake] v2.7.0 +* **Gradle plugin**: `com.docktape.swagger-brake` +* **Documentation**: https://docktape.github.io/swagger-brake/ +* **License**: Apache 2.0 diff --git a/fineract-doc/src/docs/en/chapters/architecture/index.adoc b/fineract-doc/src/docs/en/chapters/architecture/index.adoc index 01a635c6f..212f8b452 100644 --- a/fineract-doc/src/docs/en/chapters/architecture/index.adoc +++ b/fineract-doc/src/docs/en/chapters/architecture/index.adoc @@ -35,3 +35,5 @@ include::business-date.adoc[leveloffset=+1] include::reliable-event-framework.adoc[leveloffset=+1] include::advanced-payment-allocation.adoc[leveloffset=+1] + +include::api-backward-compatibility.adoc[leveloffset=+1] diff --git a/fineract-document/src/main/java/org/apache/fineract/infrastructure/documentmanagement/api/ImagesApiResource.java b/fineract-document/src/main/java/org/apache/fineract/infrastructure/documentmanagement/api/ImagesApiResource.java index b29b99346..f72b6350d 100644 --- a/fineract-document/src/main/java/org/apache/fineract/infrastructure/documentmanagement/api/ImagesApiResource.java +++ b/fineract-document/src/main/java/org/apache/fineract/infrastructure/documentmanagement/api/ImagesApiResource.java @@ -197,7 +197,8 @@ public class ImagesApiResource { @PUT @Consumes(MediaType.MULTIPART_FORM_DATA) - @RequestBody(description = "Update image", content = { @Content(mediaType = MediaType.MULTIPART_FORM_DATA) }) + @RequestBody(description = "Update image", content = { + @Content(mediaType = MediaType.MULTIPART_FORM_DATA, schema = @io.swagger.v3.oas.annotations.media.Schema(type = "object")) }) public ImageCreateResponse updateImage(@PathParam(DOCUMENT_API_PARAM_ENTITY_TYPE) final String entityName, @PathParam(DOCUMENT_API_PARAM_ENTITY_ID) final Long entityId, @HeaderParam(CONTENT_LENGTH) final Long fileSize, @FormDataParam(DOCUMENT_API_PARAM_FILE) final InputStream inputStream, diff --git a/fineract-provider/build.gradle b/fineract-provider/build.gradle index e5751e020..4ab05300f 100644 --- a/fineract-provider/build.gradle +++ b/fineract-provider/build.gradle @@ -27,6 +27,26 @@ apply plugin: 'io.swagger.core.v3.swagger-gradle-plugin' apply plugin: 'com.google.cloud.tools.jib' apply plugin: 'org.springframework.boot' apply plugin: 'se.thinkcode.cucumber-runner' +apply plugin: 'com.docktape.swagger-brake' + +swaggerBrake { + newApi = "${project.buildDir}/resources/main/static/fineract.json" + oldApi = findProperty('apiBaseline') ?: "${projectDir}/config/swagger/fineract-baseline.json" + outputFormats = ['JSON'] + outputFilePath = "${project.buildDir}/swagger-brake" + deprecatedApiDeletionAllowed = true + strictValidation = false +} + +checkBreakingChanges.dependsOn resolve +checkBreakingChanges.onlyIf { + def baseline = findProperty('apiBaseline') ?: "${projectDir}/config/swagger/fineract-baseline.json" + def exists = file(baseline).exists() + if (!exists) { + logger.lifecycle("Skipping checkBreakingChanges: baseline file not found at ${baseline}") + } + exists +} check.dependsOn('cucumber')