ME-29: Enable REST-triggered export jobs with named collections & ZIPdownload

[wikumChamith]

Description of what I changed

• Add export packages: named, persisted definitions of what to export (domain + optional item uuids per entry), with a versioned build history
• Add REST endpoints under /ws/rest/v1/metadataexport to manage packages, trigger builds, poll status, and download the result as a ZIP
• Run builds async on a daemon thread (QUEUED → RUNNING → COMPLETED/FAILED); each ZIP contains the Initializer configuration/ tree plus a package.json manifest
• Add MetadataExportService, DAO, entities + liquibase tables, ExportPackageValidator, ExporterService.exportSeeds, DomainExporter.getInstancesByUuids, ZipUtils
• Add two privileges; remove the dead AdminList extension stub; 180 tests (unit, context-sensitive, MockMvc)

Issue I worked on

see https://openmrs.atlassian.net/browse/ME-29

Checklist: I completed these to help reviewers :)

• My IDE is configured to follow the code style of this project.

  No? Unsure? -> configure your IDE, format the code and add the changes with git add . && git commit --amend

• I have added tests to cover my changes. (If you refactored
  existing code that was well tested you do not have to add tests)

  No? -> write tests and add them to this commit git add . && git commit --amend

• I ran mvn clean package right before creating this pull request and
  added all formatting changes to my commit.

  No? -> execute above command

• All new and existing tests passed.

  No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.

• My pull request is based on the latest changes of the master branch.

  No? Unsure? -> execute command git pull --rebase upstream master

[wikumChamith]

I decided to deviate from our usual web services REST resources here, since most of these controllers aren't CRUD resources. I also wanted to try something new :) . If this isn't suitable, I'll switch to web services resources.

[dkayiwa]

Keep the plain controllers. Trigger, poll and download are not resources, and expressing them as webservices.rest actions would cost more machinery than the CRUD half of this API saves. Splitting across both mechanisms would be worse than either one on its own.

I did check the thing that worried me about sitting under /ws/rest/v1 without depending on the module that owns that prefix. webservices.rest maps /{resource}, /{resource}/{uuid}, /{resource}/search/{searchHandlerId} and the MainSubResourceController set, all with two to four path variables, so each literal /metadataexport/... path here is strictly more specific and wins the match whether or not that module is installed. No collision either way.

The one piece I would align is the error body. Everything else under this prefix answers with {"error": {"message": ..., "code": ..., "detail": ..., "rawMessage": ..., "translatedMessage": ...}} (RestUtil.wrapErrorResponse), while MetadataExportControllerAdvice answers with {"error": "<string>"} and {"error": "Validation failed", "fieldErrors": {...}}, so a client that already parses /ws/rest/v1 errors has to special-case these paths. Nesting the message under error is a few lines. Worth noting that its translatedMessage is exactly what the messages_es/fr thread is reaching for.

[Bawanthathilan]

Is this an i18n translation? If so, do we need to translate it?

[dkayiwa]

Answering the translation half: as written, translating these wouldn't change anything a client sees. ValidateUtil.validate does resolve the codes through the message source when it builds the ValidationException message, but MetadataExportControllerAdvice.handleValidation ignores e.getMessage() and reports FieldError.getDefaultMessage(), which is the hardcoded English third argument handed to errors.rejectValue(...). So the REST body is English whatever the request locale is.

Two ways out: resolve fieldError.getCode() through Context.getMessageSourceService() in the advice, after which the es/fr files start mattering, or drop the three keys and keep the default messages. For what it's worth messages_es.properties and messages_fr.properties were already byte-identical English copies of the English file before this PR (metadataexport.title), so untranslated content here isn't new, but adding keys to them does imply someone translated them.

[dkayiwa]

One concrete case worth folding into whichever way this goes: core's own validation errors arrive with a null defaultMessage, so those 400s carry no reason at all. A 300-character name, driven through the real controller:

status 400
{"error":"Validation failed","fieldErrors":{"name":null},"globalErrors":[]}

The FieldError behind it has code = error.exceededMaxLengthOfField and arguments = [255], and core ships that key translated already (messages.properties, messages_es.properties). So resolving the code instead of the default message fixes the null and the localisation together. Dropping the three module keys and keeping the default messages, which was the other option I floated, would leave this case reporting null.

[dkayiwa]

This needs fixing before merge. The path built here starts at /ws, but OpenMRS is served under a context path (/openmrs in every standard deployment), so a client that resolves this against the request URI lands on http://host/ws/rest/v1/metadataexport/builds/<uuid>/download and gets a 404.

I ran a MockMvc request whose context path was /openmrs, and the response still came back as:

"downloadUrl":"/ws/rest/v1/metadataexport/builds/69ea5793-218c-48a7-bdbc-9d83f33909f7/download"

ExportBuildControllerTest can't catch it, since it only asserts the value endsWith("/builds/" + uuid + "/download").

I'd derive the prefix from the request instead of hardcoding it, roughly:

dto.setDownloadUrl(ServletUriComponentsBuilder.fromCurrentContextPath()
        .path("/ws").path(MetadataExportRestConstants.BASE)
        .path("/builds/" + build.getUuid() + "/download").toUriString());

That also lines up with webservices.rest, which prepends RestConstants.URI_PREFIX (from the webservices.rest.uriPrefix global property) precisely so the link it emits resolves. If you'd rather not depend on a request being in scope, dropping the field and letting clients compose the URL is a fine second choice, but a link that 404s is worse than no link.

[dkayiwa]

This needs fixing before merge. ExportPackageRequest.entries initialises to an empty list, so Jackson leaves it empty when the field is simply absent from the body, and this clear() then wipes what was stored. Together with runBuild's "no entries = every registered domain", a client that PUTs {"name":"Renamed"} to rename a package converts a two-location package into a full-metadata export, with a 200 and nothing in the log.

Driving the real controller:

before: entries=1
PUT {"name":"Renamed ..."}  ->  200
after:  entries=0
body:   {..., "description":null, "entries":[], ...}

description gets nulled the same way.

The smallest fix I'd take is to default ExportPackageRequest.entries to null and have ExportPackageValidator reject null, with the message telling callers to send [] when they mean every domain. A partial PUT then gets a 400 rather than a quietly wider export. If you want entries to stay optional, then the "empty means everything" overload has to go instead, because that overload is what turns a dropped field into an escalation.

[dkayiwa]

Worth settling before merge: this hands the entire zip to anyone holding Manage Metadata Export Packages, and for an entries-less package that zip carries every global property value verbatim. I built one against a test database and read the archive back:

entry = configuration/globalproperties/globalProperties.xml
contains "mail.password"     = true
contains its value           = true

mail.password ("Password for the SMTP user") is seeded by core in every install, and distros keep API keys and sync credentials in global properties too. GlobalPropertyDomainExporter filters only the .started / .mandatory module-state properties, so values go out as they are.

Reading global property values normally needs Manage Global Properties, so as it stands this privilege is a way around that boundary. Requiring GET for getBuild but MANAGE for the download says you already treated the download as the sensitive half; I'd finish that thought, either by leaving GLOBAL_PROPERTIES out of the implicit all-domains export, or by also requiring Manage Global Properties when the build contains them. If you judge the new privilege admin-equivalent anyway, then please say so in the README next to the CSRF paragraph, so nobody grants it casually.

[wikumChamith]

I could add a Manage Global Properties check on both trigger and download when the build includes global properties. However, this problem could be broader than just GP. We might have to let each exporter declare the privileges it needs, but that would need to be a separate PR. @ibacher, what do you think?

[dkayiwa]

For this PR the narrow version is enough from my side: either leave GLOBAL_PROPERTIES out of the implicit all-domains export, or require Manage Global Properties on trigger and download when the resolved build contains them. Both fit inside this PR and neither pre-empts the general design.

I would not hold the PR for the per-exporter privilege declaration. It is the right shape long term, but it needs a decision on what a domain's privilege even is (the read privilege of the underlying service? a new one per domain? something coarser for the whole export?), and that is its own conversation. If you would rather not touch the export set at all right now, the README note is the minimum I would want: whoever grants Manage Metadata Export Packages needs to know it hands over every global property value.

[dkayiwa]

Not blocking, but this catch-all is reached before DefaultHandlerExceptionResolver, so Spring's own request-shape exceptions never get their normal status and malformed input comes back as a server error. Measured against the same standalone setup the tests use:

request	status now	expected
{ this is not json	500	400
empty body	500	400
Content-Type: text/plain	500	415

Each one also logs a stack trace at ERROR, so a sloppy client reads as a server fault. Handlers for HttpMessageNotReadableException and HttpMediaTypeNotSupportedException, or extending ResponseEntityExceptionHandler and keeping this as the last resort, would cover it. (PUT on a collection URL does still come back 405, since no handler method is matched at all.)

[dkayiwa]

Not blocking, but nothing dedupes itemUuids, and liquibase changeset metadataexport-2026-07-24-1000-3 puts UNIQUE(entry_id, item_uuid) on the collection table. So "itemUuids": ["x", "x"] writes two rows, fails on a real database, and surfaces through the advice as a 500.

CI can't see this one: the context-sensitive tests build their schema from hbm2ddl=create-drop and the @ElementCollection declares no unique constraint, so the test database stores both rows happily (I checked, the reloaded list comes back [dup, dup]). Wrapping the incoming list in a LinkedHashSet here, or rejecting duplicates in ExportPackageValidator so it becomes a 400, would close it.

[dkayiwa]

Nothing here blocks, but two things in this loop are worth a look.

No exporter overrides this method, so scoping a package entry to two concepts runs Context.getConceptService().getAllConcepts() and throws the rest away. On a real dictionary that is the whole concept table loaded to export two rows, which is the case export packages exist for. A per-exporter getInstancesByUuid hook (getConceptByUuid, getLocationByUuid, ...) with this scan kept as the fallback would keep the narrow case narrow.

Separately, the match is on the bare uuid, while identityKey a few lines up deliberately keys on (class, uuid) because uuids are only unique per table. For two same-uuid objects inside one domain, the case your own Javadoc names for ATTRIBUTE_TYPES, wanted.remove succeeds on the first, so the second is silently dropped and the caller is told the uuid was found. Collecting every match and tracking matched uuids in a separate set handles it.

[wikumChamith]

@dkayiwa I've updated the PR.

[dkayiwa]

Nothing here blocks merge, but the (package_id, version) unique constraint lives only in liquibase, so the concurrency guard in ExportJobRunner.trigger has never actually run. ExportPackageEntry just picked up a matching @UniqueConstraint for the item-uuid table, and this table needs the same.

Two builds at version 1 of the same package save happily against the schema the tests build:

two builds at version 1 SAVED OK -> no unique constraint in the test schema
builds now = 2

trigger's own comment names that constraint as what "catches the loser" of two concurrent triggers, so the branch that turns the violation into a 409 (ExceptionUtils.indexOfType(e, ConstraintViolationException.class)) is unreachable from any test. Adding uniqueConstraints = @UniqueConstraint(columnNames = { "package_id", "version" }) here, plus the import, lines the test schema up with changeset metadataexport-2026-07-24-1000-4 and makes the race coverable.

[dkayiwa]

GET /packages reads the whole build history of every package, manifest_json CLOB included, to use a single row of it. Not a merge blocker, but it degrades with every build. Measured over 4 packages holding 16 build rows between them:

GET /packages status       = 200
response bytes             = 2138
HQL/criteria queries       = 9        (1 + 2 per package)
JDBC prepared statements   = 16
manifest bytes read        = 311025   (15 x 20735)

20735 is one whole-registry manifest against the standard test dataset. It grows with the number of items exported, so on a real dictionary a single manifest is megabytes, and this method pulls one per historical build of every package on every list call. ExportBuildDto.from does not even expose the manifest (only detailFrom does), so every byte of it is read and thrown away.

A getLatestBuild(ExportPackage) on the DAO with setMaxResults(1) closes both halves: one query per package instead of two, and one manifest instead of the whole history. It also drops the redundant getPackageByUuid that MetadataExportService.getBuilds runs before each lookup.

[dkayiwa]

This accepts any of Initializer's 58 Domain values, but only the 27 with an exporter in this module can actually be built, so an unsupported domain is stored happily and fails much later:

POST /packages {"name":"Forms pkg","entries":[{"domain":"HTML_FORMS","itemUuids":[]}]}  ->  201
runBuild  ->  APIException: No exporter registered for domain HTML_FORMS

The client gets a 201 on create, a 202 on the trigger, and only finds out when the build lands in FAILED. Optional, but cheap to close: GET /domains already publishes the supported list, so injecting DomainExporterRegistry next to the DAO and rejecting forDomain(entry.getDomainEnum()) == null makes it a 400 at create time. The metadataexport.package.entry.domain.unknown rejection below is already the right shape to extend.

[wikumChamith]

@dkayiwa I've updated the PR.

[dkayiwa]

Retiring a package permanently burns its name, and I would rather see that settled while the changeset is still unreleased. This check does not exclude retired packages, and changeset metadataexport-2026-07-24-1000-1 also puts unique="true" on metadataexport_package.name, so the name is taken at both levels.

Driving the real controller:

POST   /packages {"name":"Site A locations", ...}   ->  201
DELETE /packages/<uuid>?reason=obsolete             ->  204
GET    /packages                                    ->  []
POST   /packages {"name":"Site A locations", ...}   ->  400
   {"error":"Validation failed","fieldErrors":{"name":"An export package with this name already exists"},"globalErrors":[]}

So the 400 names a package the default listing does not show, and since there is no unretire endpoint the only way back is to find it with ?includeRetired=true and PUT it under some other name to release the old one.

What I would do: filter retired rows out of getPackageByName (this validator is its only caller) and drop unique="true" from the name column, so retire behaves like a delete from a client's point of view. That is a one-line edit to the changeset today; once this ships it needs a second changeset to drop the index. If you would rather keep names globally unique, the rejection message needs to say the conflicting package is retired, and there has to be a way to unretire it, otherwise the name is simply lost.

[wikumChamith]

@dkayiwa, what's going to happen if we want to unretire a package and there's a new one with the same name?

[dkayiwa]

Core already answers this, and the answer is that the unretire has to fail. EncounterTypeValidator rejects a by-name duplicate only when it is a different uuid and not retired, and unretireEncounterType just flips the flag and routes back through saveEncounterType, where RequiredDataAdvice.before runs ValidateUtil.validate. So unretiring into a name that an active row now holds comes back as the duplicate-name error, and whoever wants both has to rename one of them first. That is the behaviour I would copy: once there is an unretire path here it goes through saveExportPackage anyway, so this validator covers it for free.

It is also why the check belongs here rather than in the schema. A validator can say "only active packages count"; a unique index cannot.

Core's own encounter_type.name does carry unique="true" though, so encounter types have exactly the trap I am asking you to avoid: a retired-filtered lookup, and an insert that still dies on the index. location, visit_type, concept_class, patient_identifier_type and program carry no unique constraint on name at all, and that majority shape is the one worth copying.

So concretely, and none of the unretire part needs to land in this PR:

• drop unique="true" from the name column, which is the only piece that gets expensive after release
• filter retired = false in getPackageByName, and keep a !sameName.getRetired() guard here alongside it
• if unretire shows up later, it comes back 400 from this validator saying the name is taken, which is a fair thing for a client to be told

[dkayiwa]

Whether the unretire gets checked depends on how it is written, so "for free" above was wrong of me. RequiredDataAdvice.before calls ValidateUtil.validate only on the save*/create* branch; the void/unvoid/retire/unretire branch just runs the handlers and returns. I filtered retired = false into getPackageByName, added an unretireExportPackage(pkg, reason) written exactly like the retireExportPackage you already have (straight to metadataExportDao.savePackage), and ran both shapes:

unretire via dao.savePackage, mirroring retireExportPackage
    -> ALLOWED, retired=false, 2 active rows named "Site A locations"
unretire via saveExportPackage, which is what core's unretireEncounterType does
    -> REJECTED, name=An export package with this name already exists

So the answer I gave holds, the unretire fails and whoever wants both renames one first, but only if unretire flips the flag and routes back through saveExportPackage. Written the way retire is written here it quietly leaves you with two active packages sharing a name.

Changeset metadataexport-2026-07-24-1000-1 declares metadataexport_package.name unique while ExportPackage carries no matching @UniqueConstraint, since name arrives from BaseOpenmrsMetadata as a plain @Column(name = "name", nullable = false, length = 255). Two packages with the same name save happily through the DAO in a context-sensitive test:

two packages named "Same name" via the DAO: SAVED OK -> no unique index in the test schema

That is the same entity-versus-liquibase drift you closed for the item-uuid table and for (package_id, version), so this is the third one, and it bites here specifically: a test asserting that a retired package's name can be reused will pass in CI and then 500 in production for as long as the changeset disagrees. Worth lining the two up whichever way you decide the naming question.

Correcting the precedent I cited too: encounter_type.name does carry unique="true" in core's schema, so encounter types cannot actually reach the state you asked about, the insert dies on the index before any validator gets a say. location, visit_type, concept_class, patient_identifier_type and program carry no unique constraint on name, and the 2.8.x snapshot holds only three standalone unique constraints, none on a name column. The validator-side check is the part core standardises; the unique index on encounter_type is the outlier, not the model.

If you do write a test for this, flush between the retire and the recreate. Same code both times, the flush is the only difference:

no flush between retire and recreate  ->  recreate REJECTED
explicit flush                        ->  recreate ACCEPTED

Over REST those are two separate requests so it never comes up, but inside one test transaction it looks exactly like the fix not working.

[wikumChamith]

Done!

[dkayiwa]

Should a retired package still be buildable? As it stands, DELETE /packages/{uuid} followed by POST /packages/{uuid}/builds returns 202 and runs the full export against a package that GET /packages no longer lists:

DELETE /packages/<uuid>?reason=obsolete  ->  204
GET    /packages                         ->  []
POST   /packages/<uuid>/builds           ->  202  {"version":1,"status":"QUEUED", ...}

If that is deliberate, so an admin can still produce an export of something they have shelved, there is nothing to do here. If not, a getRetired() check beside the null check above, answering 409 with the reason, would close it. Either way GET /packages/{uuid} still returning the retired package looks right to me.

[dkayiwa]

@wikumChamith do you know what is going on with the above build failures?

[wikumChamith]

@wikumChamith do you know what is going on with the above build failures?

Just a spotless error. I fixed it :)

[dkayiwa]

This advice is never consulted on a platform + Initializer install, so none of the statuses or JSON bodies the README documents actually happen there. Core's openmrs-servlet.xml registers exactly one HandlerExceptionResolver, a SimpleMappingExceptionResolver mapping java.lang.Exception to the uncaughtException view, and DispatcherServlet only falls back to Spring's defaults (which include ExceptionHandlerExceptionResolver) when it finds none at all. So nothing invokes @RestControllerAdvice.

Driving the real chain instead of a standalone one (webAppContextSetup over the context BaseModuleWebContextSensitiveTest builds from classpath*:openmrs-servlet.xml plus this module's own webModuleApplicationContext.xml):

ExceptionHandlerExceptionResolver beans = []

GET  /packages                   -> 200 []
GET  /packages/no-such-uuid      -> 404
POST duplicate name              -> InvalidDefinitionException propagates out of the DispatcherServlet
POST unknown domain              -> InvalidDefinitionException propagates out of the DispatcherServlet
POST malformed json              -> InvalidDefinitionException propagates out of the DispatcherServlet
POST without entries             -> 200, body = {"cause":null,"stackTrace":[{...ExportPackageController...
GET  /packages while logged out  -> 200, body = {"cause":null,"stackTrace":[{...Context.requirePrivilege...

The last two are why I think this blocks: a client that checks the status code reads a privilege failure as success, and what comes back is a server stack trace. The three that propagate are MappingJackson2JsonView choking on ValidationException.errors while rendering uncaughtException, so an intended 400 becomes whatever the container does with an unhandled exception.

A Reference Application never shows this because of webservices.rest. Its omod-common/src/main/resources/webModuleApplicationContext.xml has <mvc:annotation-driven/>, which registers ExceptionHandlerExceptionResolver, ResponseStatusExceptionResolver and DefaultHandlerExceptionResolver, and OpenMRS loads every module's webModuleApplicationContext.xml into the root context. legacyui contributes none. So the README's "the webservices.rest module is not required" holds for routing, since /ws/* is core's own servlet mapping, but not for error handling.

I would register the resolver in the omod's webModuleApplicationContext.xml, which already exists for the component scan. Two properties matter, and I had both wrong on my first attempt: order has to sit below SimpleMappingExceptionResolver's 100 or that one still wins, and the converters have to be supplied or writing the ResponseEntity body fails, leaving you with the right status and a stack-trace body anyway.

<bean class="org.springframework.web.servlet.mvc.method.annotation.ExceptionHandlerExceptionResolver">
    <property name="order" value="0" />
    <property name="messageConverters">
        <list>
            <ref bean="jacksonMessageConverter" />
        </list>
    </property>
</bean>

jacksonMessageConverter is core's own bean from openmrs-servlet.xml, so nothing new is needed. With that in the same harness:

POST duplicate name              -> 400 {"error":"Validation failed","fieldErrors":{"name":"An export package with this name already exists"},"globalErrors":[]}
POST unknown domain              -> 400 {"error":"Validation failed","fieldErrors":{"entries[0].domain":"Unknown domain 'NOT_A_DOMAIN'"},"globalErrors":[]}
POST malformed json              -> 400
POST without entries             -> 400 {"error":"entries is required; send an empty list to export every registered domain"}
GET  /packages while logged out  -> 401 {"error":"error.privilegesRequired"}

Declaring webservices.rest in require_modules and correcting the README is the other way out, defensible since the curl examples already lean on its auth filter, but I would rather the module stood on its own.

Worth converting one test case to webAppContextSetup(wac) while you are in there. Every 400/415/409 assertion in the suite passes today because standaloneSetup(...).setControllerAdvice(...) hands the tests a resolver production does not have, so nothing in CI can see any of this.

[dkayiwa]

item_uuid is char(38) not null in changeset metadataexport-2026-07-24-1000-3, but this column declares neither a length nor nullable = false, so the schema the tests build is varchar(255) and nullable. Nothing validates itemUuids, so both mismatches are reachable straight from a request body:

test schema: METADATAEXPORT_PACKAGE_ENTRY_ITEM.ITEM_UUID  VARCHAR_IGNORECASE  len=255  nullable=YES

61-char itemUuid through saveExportPackage  -> ACCEPTED, reloads exactly as sent
the same value into a char(38) column       -> Value too long for column "ITEM_UUID CHAR(38)"
[null] itemUuid through saveExportPackage   -> ACCEPTED

Nothing here blocks merge, it takes a malformed uuid to reach, but a client that puts a name or a URL in itemUuids gets a 201 in CI and a data-truncation error against a real MySQL. length = 38 would line the two schemas up the way uuid already does through BaseOpenmrsObject, though on its own it only moves the failure to a 500, so I would also reject over-long and null entries in ExportPackageValidator so it lands as a 400 beside the domain checks. domain, version, package_id and export_status drift on nullability too, but those are either set by module code or already caught by the validator, so item_uuid is the one a caller can actually reach.

[wikumChamith]

@dkayiwa I've updated the PR.

[dkayiwa]

description is varchar(255) here while the entity maps it as a CLOB, and an ordinary long description reaches that mismatch straight from a request body, so this wants fixing before merge. BaseOpenmrsMetadata declares the field @Column(length = 255) and @Lob, so hbm2ddl builds the column as a CLOB. Nothing rejects the value on the way in either: HibernateAdministrationDAO.validate only length-checks properties whose Hibernate type is StringType or TextType, which is exactly why a 300-character name comes back 400 and a 300-character description does not.

Through the real dispatcher chain:

POST /packages, description = 300 chars  ->  201
reloaded from the database               ->  300 chars, byte identical

Into the column this changeset actually creates, on MySQL 8.0.46 with the sql_mode it ships with:

STRICT_TRANS_TABLES (the default)  ->  ERROR 1406 (22001): Data too long for column 'description'
sql_mode=''                        ->  stored, length 255

So a description longer than a couple of sentences is a 201 in CI and then either a 500 from the catch-all advice or a quietly truncated description, depending on how the site's MySQL is configured. This one needs no malformed input at all, which also makes "item_uuid is the one a caller can actually reach" from my earlier comment wrong.

clob lines the changeset up with the entity, and it is already the type error_message and manifest_json use in changeset -4:

Suggested change

	<column name="description" type="varchar(255)"/>
	<column name="description" type="clob"/>

Cheap while the changeset is still unreleased; afterwards it needs a second one. A length check in ExportPackageValidator would close the 500, but the mapping would still say unbounded, so CI would not catch the next regression either.

[dkayiwa]

The bean lands in the shared OpenMRS context rather than a module-private one, so it becomes the application's ExceptionHandlerExceptionResolver rather than this module's. Not a merge blocker, but worth a one-value change.

web.xml loads webModuleApplicationContext.xml, applicationContext-service.xml and openmrs-servlet.xml into a single context, and @RestControllerAdvice(basePackages = ...) does not narrow the resolver: basePackages filters advice beans only, while the resolver still picks up @ExceptionHandler methods declared inside whatever controller threw. Driving resolveException with a handler method from a controller in another package that declares its own handler, it handled it and wrote the body.

On a platform install that costs nothing, since core and legacyui declare no @ExceptionHandler or @ControllerAdvice at all. It matters beside webservices.rest: its omod-common/webModuleApplicationContext.xml carries <mvc:annotation-driven/>, which registers its own ExceptionHandlerExceptionResolver at order 0 too, and its BaseRestController has five controller-local @ExceptionHandler methods. Two resolvers at the same order means bean registration order picks the winner, and this one holds a single converter:

this bean                               -> MappingJackson2HttpMessageConverter [application/json, application/*+json]
core's own RequestMappingHandlerAdapter -> six, including String, Form, Source and the XStream marshaller

Under Accept: application/json the two are indistinguishable, so most traffic never notices. Under anything else this one invokes the handler method and then declines, and the request falls through to uncaughtException carrying whatever that method already did to the response (webservices.rest's handlers set the status before returning).

A higher order value makes it deterministic. webservices.rest's resolver then wins wherever it is installed, and it finds MetadataExportControllerAdvice anyway since it scans the same context, while this one still beats SimpleMappingExceptionResolver at 100 on the platform-only install it was added for.

Suggested change

	<property name="order" value="0" />
	<property name="order" value="10" />

The 0 came out of the snippet I gave you in the advice thread, so that part is on me.

[dkayiwa]

@wikumChamith looks good for merging 👍