Add deprecated backward-compatible ExtraMetadata API adapters#14133
Add deprecated backward-compatible ExtraMetadata API adapters#14133Copilot wants to merge 5 commits intocopilot/refactor-extra-metadata-fieldfrom
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @Copilot on file. In order for us to review and merge your code, please contact the project maintainers to get yourself added. |
Add isolated deprecated adapters that re-expose the old /extra_metadata/ endpoint and metadata serializer field using SparseField as storage. - New file: deprecated_extra_metadata.py (self-contained, easy to remove) - DeprecatedExtraMetadataMixin: restores GET/PUT/POST/DELETE endpoint - DeprecatedExtraMetadataField: restores metadata deferred field - All marked @deprecated(version="4.4.0") with deprecation warnings - Tests covering all CRUD operations and serializer field Agent-Logs-Url: https://github.com/GeoNode/geonode/sessions/39c9c964-1dcf-4194-8452-fac2b2f27292 Co-authored-by: etj <717359+etj@users.noreply.github.com>
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @Copilot on file. In order for us to review and merge your code, please contact the project maintainers to get yourself added. |
Agent-Logs-Url: https://github.com/GeoNode/geonode/sessions/39c9c964-1dcf-4194-8452-fac2b2f27292 Co-authored-by: etj <717359+etj@users.noreply.github.com>
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @Copilot on file. In order for us to review and merge your code, please contact the project maintainers to get yourself added. |
There was a problem hiding this comment.
Pull request overview
Restores the removed legacy ExtraMetadata API surface as deprecated, backward-compatible adapters backed by SparseField, providing external API consumers a migration window while keeping the new sparse-fields storage model.
Changes:
- Added a new isolated adapter module implementing a deprecated
/{pk}/extra_metadataCRUD action and a deprecated serializermetadatafield reconstructed fromSparseField. - Integrated the adapter via a mixin on
ResourceBaseViewSetand a deferred computed field onResourceBaseSerializer. - Added API tests covering CRUD behavior and serializer output.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
geonode/base/api/deprecated_extra_metadata.py |
New deprecated adapter module implementing legacy endpoint + serializer field using SparseField storage. |
geonode/base/api/views.py |
Adds the deprecated endpoint back by mixing DeprecatedExtraMetadataMixin into ResourceBaseViewSet. |
geonode/base/api/serializers.py |
Reintroduces deprecated metadata as a deferred computed field. |
geonode/base/api/tests.py |
Adds tests covering deprecated endpoint CRUD and serializer field reconstruction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| To remove these adapters: | ||
| 1. Delete this file. | ||
| 2. Remove the ``metadata`` field and import from ``serializers.py``. | ||
| 3. Remove the ``extra_metadata`` action and import from ``views.py``. |
There was a problem hiding this comment.
The removal checklist in the module docstring says to remove the "extra_metadata" action from views.py, but the integration is via adding DeprecatedExtraMetadataMixin to ResourceBaseViewSet (the action itself lives in this module). Update step 3 to reflect removing the mixin from views.py to avoid misleading future removal work.
| 3. Remove the ``extra_metadata`` action and import from ``views.py``. | |
| 3. Remove ``DeprecatedExtraMetadataMixin`` and its import from ``views.py``. |
| @deprecated(version=DEPRECATION_VERSION, reason=DEPRECATION_REASON) | ||
| def get_attribute(self, instance): | ||
| warnings.warn(DEPRECATION_REASON, DeprecationWarning, stacklevel=2) | ||
| try: | ||
| qs = _sparse_fields_for_resource(instance) | ||
| return [_sparse_to_legacy(sf) for sf in qs] | ||
| except Exception as e: | ||
| logger.exception(e) | ||
| return [] |
There was a problem hiding this comment.
PR description says all adapter paths emit both a DeprecationWarning and a log warning, but DeprecatedExtraMetadataField.get_attribute() only calls warnings.warn(...) and never logs. If you want consistent observability with the endpoint adapter, add a logger.warning(...) here (or adjust the PR description if logging is intentionally omitted for serializer usage).
| qs = _sparse_fields_for_resource(resource) | ||
| # Support the old query-param filtering (e.g. ?field_name=value) | ||
| for key, value in request.query_params.items(): | ||
| # Old API used metadata__<key>=value JSONField lookups. | ||
| # We approximate this by filtering on the JSON string. | ||
| filtered = [] | ||
| for sf in qs: | ||
| try: | ||
| meta = json.loads(sf.value) if sf.value else {} | ||
| except (json.JSONDecodeError, TypeError): | ||
| continue | ||
| if str(meta.get(key)) == str(value): | ||
| filtered.append(sf) | ||
| qs = filtered | ||
| break # Old API only used the first filter pair | ||
|
|
There was a problem hiding this comment.
_extra_metadata_get applies legacy filtering to the first query param unconditionally. This can produce incorrect results if clients include unrelated params (e.g. api_preset, page_size, format, etc.), because the loop will treat them as a metadata filter and then break. Consider whitelisting filter keys (or explicitly skipping known non-filter params) before applying the legacy filter logic.
| for meta_dict in data: | ||
| if not isinstance(meta_dict, dict): | ||
| continue | ||
| meta_dict.pop("id", None) | ||
| value = json.dumps(meta_dict) | ||
| if len(value) > SPARSE_FIELD_VALUE_MAX_LENGTH: | ||
| logger.warning( | ||
| "extra_metadata entry skipped: serialized value exceeds " | ||
| f"{SPARSE_FIELD_VALUE_MAX_LENGTH} characters" | ||
| ) | ||
| continue | ||
| name = _next_sparse_name(resource) | ||
| SparseField.objects.create( | ||
| resource=resource, name=name, value=value | ||
| ) |
There was a problem hiding this comment.
Creating new SparseField entries uses _next_sparse_name(resource) + SparseField.objects.create(...), but SparseField has unique_together = (resource, name). Under concurrent POSTs, two requests can compute the same next name and one will raise an IntegrityError (500). Consider using a retry loop on IntegrityError, or generating a collision-resistant name (e.g., include a UUID) while still keeping the extra_metadata_ prefix.
| pk = pk or self.dataset.pk | ||
| return urljoin( | ||
| f"{reverse('base-resources-list')}/", | ||
| f"{pk}/extra_metadata", |
There was a problem hiding this comment.
The test helper builds the endpoint URL without a trailing slash (.../{pk}/extra_metadata). Other ResourceBaseViewSet detail actions in this test module use a trailing slash (e.g. .../{pk}/favorite/), and routers commonly enforce the slash, returning a redirect (301) instead of 200/201. Consider using reverse('base-resources-extra-metadata', args=[pk]) or appending a trailing / here to match the router.
| f"{pk}/extra_metadata", | |
| f"{pk}/extra_metadata/", |
|
@copilot mapstore-client still relies on EXTRA_METADATA_SCHEMA, so removing it from the settings is absolutely not back compatible. Here the error log at boot time: |
…comments - Restore EXTRA_METADATA_SCHEMA, DEFAULT_EXTRA_METADATA_SCHEMA, CUSTOM_METADATA_SCHEMA settings with deprecation markers (required by mapstore-client) - Restore 'from schema import Optional' import in settings.py - Fix docstring removal checklist (step 3 + add step 4 for settings) - Add logger.warning to DeprecatedExtraMetadataField.get_attribute() - Fix query param filtering to skip known non-filter params - Handle IntegrityError on concurrent POST via UUID fallback - Fix trailing slash in test URL helper Agent-Logs-Url: https://github.com/GeoNode/geonode/sessions/b6b2de77-7b42-4630-995a-d2c46d014e84 Co-authored-by: etj <717359+etj@users.noreply.github.com>
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @Copilot on file. In order for us to review and merge your code, please contact the project maintainers to get yourself added. |
…rams constant Agent-Logs-Url: https://github.com/GeoNode/geonode/sessions/b6b2de77-7b42-4630-995a-d2c46d014e84 Co-authored-by: etj <717359+etj@users.noreply.github.com>
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @Copilot on file. In order for us to review and merge your code, please contact the project maintainers to get yourself added. |
Restored in 9dc0a3a. The |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## copilot/refactor-extra-metadata-field #14133 +/- ##
========================================================================
Coverage ? 74.57%
========================================================================
Files ? 960
Lines ? 57849
Branches ? 7883
========================================================================
Hits ? 43143
Misses ? 12934
Partials ? 1772 🚀 New features to boost your workflow:
|
The ExtraMetadata removal PR dropped the
/extra_metadata/endpoint,metadataserializer field, andEXTRA_METADATA_SCHEMAsettings. This PR restores them as deprecated adapters backed by SparseField storage, giving external API consumers (including mapstore-client) a migration window.Design
All adapter code lives in a single isolated module (
geonode/base/api/deprecated_extra_metadata.py) with explicit removal instructions in the docstring. Integration into existing code is minimal—one mixin addition, one field addition, and restored settings with deprecation markers.Changes
deprecated_extra_metadata.py(new): Self-contained module with:DeprecatedExtraMetadataMixin— restores/{pk}/extra_metadata/(GET/PUT/POST/DELETE) using SparseField entries prefixedextra_metadata_DeprecatedExtraMetadataField— deferred computed field reconstructing legacymetadatalist from SparseFieldDeprecationWarning+ log warnings; OpenAPI schema markeddeprecated=TrueIntegrityErrorcatch with UUID fallbackapi_preset,page_size) are skipped during legacy filteringsettings.py: RestoredDEFAULT_EXTRA_METADATA_SCHEMA,CUSTOM_METADATA_SCHEMA,EXTRA_METADATA_SCHEMA, andfrom schema import Optional— required by mapstore-client at boot time. All marked with deprecation comments.views.py:ResourceBaseViewSetgainsDeprecatedExtraMetadataMixinserializers.py:ResourceBaseSerializergains deferredmetadatafieldtests.py:DeprecatedExtraMetadataApiTestcovering full CRUD + serializer outputRemoval checklist (post-deprecation)
deprecated_extra_metadata.pymetadatafield + import fromserializers.pyDeprecatedExtraMetadataMixin+ import fromviews.pysettings.py(DEFAULT_EXTRA_METADATA_SCHEMA,CUSTOM_METADATA_SCHEMA,EXTRA_METADATA_SCHEMA, and thefrom schema import Optionalimport)