From d91710552b001fa08afabdeef0101aac02f09af9 Mon Sep 17 00:00:00 2001
From: Jon Duckworth <duckontheweb@gmail.com>
Date: Tue, 31 Aug 2021 12:17:50 -0400
Subject: [PATCH 1/2] Use mutable dictionary for Asset properties

---
 pystac/asset.py                     | 122 ++++++++++++++++------------
 pystac/base.py                      |  35 ++++++++
 pystac/common_metadata.py           |  18 ++--
 pystac/extensions/datacube.py       |   2 +-
 pystac/extensions/eo.py             |   2 +-
 pystac/extensions/file.py           |   2 +-
 pystac/extensions/pointcloud.py     |   2 +-
 pystac/extensions/projection.py     |   2 +-
 pystac/extensions/raster.py         |   2 +-
 pystac/extensions/sar.py            |   2 +-
 pystac/extensions/sat.py            |   2 +-
 pystac/extensions/timestamps.py     |   2 +-
 pystac/extensions/view.py           |   2 +-
 pystac/item.py                      |   6 +-
 tests/extensions/test_custom.py     |   2 +-
 tests/extensions/test_pointcloud.py |   2 +-
 16 files changed, 127 insertions(+), 78 deletions(-)
 create mode 100644 pystac/base.py

diff --git a/pystac/asset.py b/pystac/asset.py
index 2c9582a13..6872179ec 100644
--- a/pystac/asset.py
+++ b/pystac/asset.py
@@ -1,7 +1,8 @@
 from copy import copy
 from typing import Any, Dict, List, Optional, TYPE_CHECKING, Union
 
-from pystac import common_metadata
+from pystac import base, common_metadata
+from pystac.media_type import MediaType
 from pystac import utils
 
 if TYPE_CHECKING:
@@ -10,7 +11,7 @@
     from pystac.item import Item as Item_Type
 
 
-class Asset:
+class Asset(base.JSONObject):
     """An object that contains a link to data associated with an Item or Collection that
     can be downloaded or streamed.
 
@@ -30,33 +31,13 @@ class Asset:
             object JSON.
     """
 
-    href: str
-    """Link to the asset object. Relative and absolute links are both allowed."""
-
-    title: Optional[str]
-    """Optional displayed title for clients and users."""
-
-    description: Optional[str]
-    """A description of the Asset providing additional details, such as how it was
-    processed or created. CommonMark 0.29 syntax MAY be used for rich text
-    representation."""
-
-    media_type: Optional[str]
-    """Optional description of the media type. Registered Media Types are preferred.
-    See :class:`~pystac.MediaType` for common media types."""
-
-    roles: Optional[List[str]]
-    """Optional, Semantic roles (i.e. thumbnail, overview, data, metadata) of the
-    asset."""
+    fields: Dict[str, Any]
+    """Dictionary of JSON fields for this asset."""
 
     owner: Optional[Union["Item_Type", "Collection_Type"]]
     """The :class:`~pystac.Item` or :class:`~pystac.Collection` that this asset belongs
     to, or ``None`` if it has no owner."""
 
-    extra_fields: Dict[str, Any]
-    """Optional, additional fields for this asset. This is used by extensions as a
-    way to serialize and deserialize properties on asset object JSON."""
-
     def __init__(
         self,
         href: str,
@@ -66,16 +47,78 @@ def __init__(
         roles: Optional[List[str]] = None,
         extra_fields: Optional[Dict[str, Any]] = None,
     ) -> None:
+        self.fields = extra_fields or {}
         self.href = href
         self.title = title
         self.description = description
         self.media_type = media_type
         self.roles = roles
-        self.extra_fields = extra_fields or {}
 
         # The Item which owns this Asset.
         self.owner = None
 
+    @property
+    def href(self) -> str:
+        """Link to the asset object. Relative and absolute links are both allowed."""
+        return self._get_field("href", str, required=True)
+
+    @href.setter
+    def href(self, v: str) -> None:
+        self._set_field("href", v)
+
+    @property
+    def title(self) -> Optional[str]:
+        """Optional displayed title for clients and users."""
+        return self._get_field("title", str)
+
+    @title.setter
+    def title(self, v: Optional[str]) -> None:
+        self._set_field("title", v, pop_if_none=True)
+
+    @property
+    def description(self) -> Optional[str]:
+        """A description of the Asset providing additional details, such as how it was
+        processed or created. CommonMark 0.29 syntax MAY be used for rich text
+        representation."""
+        return self._get_field("description", str)
+
+    @description.setter
+    def description(self, v: Optional[str]) -> None:
+        self._set_field("description", v, pop_if_none=True)
+
+    @property
+    def media_type(self) -> Optional[Union[MediaType, str]]:
+        """Optional description of the media type. Registered Media Types are preferred.
+        See :class:`~pystac.MediaType` for common media types."""
+        return self._get_field("type", str)
+
+    @media_type.setter
+    def media_type(self, v: Optional[Union[MediaType, str]]) -> None:
+        self._set_field("type", v, pop_if_none=True)
+
+    @property
+    def roles(self) -> Optional[List[str]]:
+        """Optional, Semantic roles (i.e. thumbnail, overview, data, metadata) of the
+        asset."""
+        return self._get_field("roles", List[str])
+
+    @roles.setter
+    def roles(self, v: Optional[List[str]]) -> None:
+        self._set_field("roles", v, pop_if_none=True)
+
+    @property
+    def extra_fields(self) -> Dict[str, Any]:
+        return {
+            k: v
+            for k, v in self.fields.items()
+            if k not in {"href", "title", "description", "type", "roles"}
+        }
+
+    @extra_fields.setter
+    def extra_fields(self, v: Dict[str, Any]) -> None:
+        for _k, _v in v.items():
+            self._set_field(_k, _v, pop_if_none=True)
+
     def set_owner(self, obj: Union["Collection_Type", "Item_Type"]) -> None:
         """Sets the owning item of this Asset.
 
@@ -104,33 +147,6 @@ def get_absolute_href(self) -> Optional[str]:
             else:
                 return None
 
-    def to_dict(self) -> Dict[str, Any]:
-        """Generate a dictionary representing the JSON of this Asset.
-
-        Returns:
-            dict: A serialization of the Asset that can be written out as JSON.
-        """
-
-        d: Dict[str, Any] = {"href": self.href}
-
-        if self.media_type is not None:
-            d["type"] = self.media_type
-
-        if self.title is not None:
-            d["title"] = self.title
-
-        if self.description is not None:
-            d["description"] = self.description
-
-        if self.extra_fields is not None and len(self.extra_fields) > 0:
-            for k, v in self.extra_fields.items():
-                d[k] = v
-
-        if self.roles is not None:
-            d["roles"] = self.roles
-
-        return d
-
     def clone(self) -> "Asset":
         """Clones this asset.
 
@@ -144,7 +160,7 @@ def clone(self) -> "Asset":
             description=self.description,
             media_type=self.media_type,
             roles=self.roles,
-            extra_fields=self.extra_fields,
+            extra_fields=dict(self.extra_fields),
         )
 
     @property
diff --git a/pystac/base.py b/pystac/base.py
new file mode 100644
index 000000000..152edfbe1
--- /dev/null
+++ b/pystac/base.py
@@ -0,0 +1,35 @@
+from copy import deepcopy
+from typing import Any, Dict, Type, TypeVar
+
+from pystac.utils import get_required
+
+
+V = TypeVar("V")
+
+
+class JSONObject:
+    """A base class for objects that can be represented as JSON objects.
+    
+    Instances of ``JSONObject`` have a :attr:`JSONObject.fields` attribute that is a
+    dictionary representation of the JSON object. Any property getters/setters on
+    inheriting classes MUST be sure to keep this dictionary up-to-date.
+    """
+    fields = {}
+
+    def to_dict(self, preserve_dict: bool = True) -> Dict[str, Any]:
+        if preserve_dict:
+            return deepcopy(self.fields)
+        else:
+            return self.fields
+    
+    def _get_field(self, name: str, _type: Type, *, required: bool = False) -> V:
+        if required:
+            return get_required(self.fields.get(name), self, name)
+        else:
+            return self.fields.get(name)
+    
+    def _set_field(self, name: str, v: Any, *, pop_if_none: bool = False) -> None:
+        if v is None and pop_if_none:
+            self.fields.pop(name, None)
+        else:
+            self.fields[name] = v
diff --git a/pystac/common_metadata.py b/pystac/common_metadata.py
index 5c43586e9..35aa2141e 100644
--- a/pystac/common_metadata.py
+++ b/pystac/common_metadata.py
@@ -1,9 +1,10 @@
 from datetime import datetime as Datetime
-from pystac.errors import STACError
 from typing import Any, cast, Dict, List, Optional, Type, TYPE_CHECKING, TypeVar, Union
 
 import pystac
 from pystac import utils
+from pystac.base import JSONObject
+from pystac.errors import STACError
 
 if TYPE_CHECKING:
     from pystac.provider import Provider as Provider_Type
@@ -32,19 +33,16 @@ def __init__(self, object: Union["Asset_Type", "Item_Type"]):
     def _set_field(self, prop_name: str, v: Optional[Any]) -> None:
         if hasattr(self.object, prop_name):
             setattr(self.object, prop_name, v)
-        elif hasattr(self.object, "properties"):
-            item = cast("Item_Type", self.object)
+        elif isinstance(self.object, pystac.Item):
             if v is None:
-                item.properties.pop(prop_name, None)
+                self.object.properties.pop(prop_name, None)
             else:
-                item.properties[prop_name] = v
-        elif hasattr(self.object, "extra_fields") and isinstance(
-            self.object.extra_fields, Dict
-        ):
+                self.object.properties[prop_name] = v
+        elif isinstance(self.object, JSONObject):
             if v is None:
-                self.object.extra_fields.pop(prop_name, None)
+                self.object.fields.pop(prop_name, None)
             else:
-                self.object.extra_fields[prop_name] = v
+                self.object.fields[prop_name] = v
         else:
             raise pystac.STACError(f"Cannot set field {prop_name} on {self}.")
 
diff --git a/pystac/extensions/datacube.py b/pystac/extensions/datacube.py
index fa51f61b1..a2168c755 100644
--- a/pystac/extensions/datacube.py
+++ b/pystac/extensions/datacube.py
@@ -528,7 +528,7 @@ class AssetDatacubeExtension(DatacubeExtension[pystac.Asset]):
 
     def __init__(self, asset: pystac.Asset):
         self.asset_href = asset.href
-        self.properties = asset.extra_fields
+        self.properties = asset.fields
         if asset.owner and isinstance(asset.owner, pystac.Item):
             self.additional_read_properties = [asset.owner.properties]
         else:
diff --git a/pystac/extensions/eo.py b/pystac/extensions/eo.py
index 3d56095ee..b71e3882b 100644
--- a/pystac/extensions/eo.py
+++ b/pystac/extensions/eo.py
@@ -455,7 +455,7 @@ def _get_bands(self) -> Optional[List[Band]]:
 
     def __init__(self, asset: pystac.Asset):
         self.asset_href = asset.href
-        self.properties = asset.extra_fields
+        self.properties = asset.fields
         if asset.owner and isinstance(asset.owner, pystac.Item):
             self.additional_read_properties = [asset.owner.properties]
 
diff --git a/pystac/extensions/file.py b/pystac/extensions/file.py
index 5a1817d60..8c2e9aff7 100644
--- a/pystac/extensions/file.py
+++ b/pystac/extensions/file.py
@@ -102,7 +102,7 @@ class FileExtension(
 
     def __init__(self, asset: pystac.Asset):
         self.asset_href = asset.href
-        self.properties = asset.extra_fields
+        self.properties = asset.fields
         if asset.owner and isinstance(asset.owner, pystac.Item):
             self.additional_read_properties = [asset.owner.properties]
 
diff --git a/pystac/extensions/pointcloud.py b/pystac/extensions/pointcloud.py
index e32029e72..d104efa2e 100644
--- a/pystac/extensions/pointcloud.py
+++ b/pystac/extensions/pointcloud.py
@@ -495,7 +495,7 @@ class AssetPointcloudExtension(PointcloudExtension[pystac.Asset]):
 
     def __init__(self, asset: pystac.Asset):
         self.asset_href = asset.href
-        self.properties = asset.extra_fields
+        self.properties = asset.fields
         if asset.owner and isinstance(asset.owner, pystac.Item):
             self.additional_read_properties = [asset.owner.properties]
             self.repr_id = f"href={asset.href} item.id={asset.owner.id}"
diff --git a/pystac/extensions/projection.py b/pystac/extensions/projection.py
index ba90adbe3..3d82d657f 100644
--- a/pystac/extensions/projection.py
+++ b/pystac/extensions/projection.py
@@ -335,7 +335,7 @@ class AssetProjectionExtension(ProjectionExtension[pystac.Asset]):
 
     def __init__(self, asset: pystac.Asset):
         self.asset_href = asset.href
-        self.properties = asset.extra_fields
+        self.properties = asset.fields
         if asset.owner and isinstance(asset.owner, pystac.Item):
             self.additional_read_properties = [asset.owner.properties]
 
diff --git a/pystac/extensions/raster.py b/pystac/extensions/raster.py
index 28077595a..057dbc510 100644
--- a/pystac/extensions/raster.py
+++ b/pystac/extensions/raster.py
@@ -651,7 +651,7 @@ class RasterExtension(
 
     def __init__(self, asset: pystac.Asset):
         self.asset_href = asset.href
-        self.properties = asset.extra_fields
+        self.properties = asset.fields
         if asset.owner and isinstance(asset.owner, pystac.Item):
             self.additional_read_properties = [asset.owner.properties]
 
diff --git a/pystac/extensions/sar.py b/pystac/extensions/sar.py
index cad6cb437..f5316cd78 100644
--- a/pystac/extensions/sar.py
+++ b/pystac/extensions/sar.py
@@ -381,7 +381,7 @@ class AssetSarExtension(SarExtension[pystac.Asset]):
 
     def __init__(self, asset: pystac.Asset):
         self.asset_href = asset.href
-        self.properties = asset.extra_fields
+        self.properties = asset.fields
         if asset.owner and isinstance(asset.owner, pystac.Item):
             self.additional_read_properties = [asset.owner.properties]
 
diff --git a/pystac/extensions/sat.py b/pystac/extensions/sat.py
index 21885e255..a009521b4 100644
--- a/pystac/extensions/sat.py
+++ b/pystac/extensions/sat.py
@@ -214,7 +214,7 @@ class AssetSatExtension(SatExtension[pystac.Asset]):
 
     def __init__(self, asset: pystac.Asset):
         self.asset_href = asset.href
-        self.properties = asset.extra_fields
+        self.properties = asset.fields
         if asset.owner and isinstance(asset.owner, pystac.Item):
             self.additional_read_properties = [asset.owner.properties]
 
diff --git a/pystac/extensions/timestamps.py b/pystac/extensions/timestamps.py
index f8adc89ea..edc1407d6 100644
--- a/pystac/extensions/timestamps.py
+++ b/pystac/extensions/timestamps.py
@@ -192,7 +192,7 @@ class AssetTimestampsExtension(TimestampsExtension[pystac.Asset]):
 
     def __init__(self, asset: pystac.Asset):
         self.asset_href = asset.href
-        self.properties = asset.extra_fields
+        self.properties = asset.fields
         if asset.owner and isinstance(asset.owner, pystac.Item):
             self.additional_read_properties = [asset.owner.properties]
 
diff --git a/pystac/extensions/view.py b/pystac/extensions/view.py
index 9b9bc08f6..8836c0472 100644
--- a/pystac/extensions/view.py
+++ b/pystac/extensions/view.py
@@ -221,7 +221,7 @@ class AssetViewExtension(ViewExtension[pystac.Asset]):
 
     def __init__(self, asset: pystac.Asset):
         self.asset_href = asset.href
-        self.properties = asset.extra_fields
+        self.properties = asset.fields
         if asset.owner and isinstance(asset.owner, pystac.Item):
             self.additional_read_properties = [asset.owner.properties]
 
diff --git a/pystac/item.py b/pystac/item.py
index 745aa3f09..71f3f28f2 100644
--- a/pystac/item.py
+++ b/pystac/item.py
@@ -168,10 +168,10 @@ def get_datetime(self, asset: Optional[Asset] = None) -> Optional[Datetime]:
         Returns:
             datetime or None
         """
-        if asset is None or "datetime" not in asset.extra_fields:
+        if asset is None or "datetime" not in asset.fields:
             return self.datetime
         else:
-            asset_dt = asset.extra_fields.get("datetime")
+            asset_dt = asset.fields.get("datetime")
             if asset_dt is None:
                 return None
             else:
@@ -186,7 +186,7 @@ def set_datetime(self, datetime: Datetime, asset: Optional[Asset] = None) -> Non
         if asset is None:
             self.datetime = datetime
         else:
-            asset.extra_fields["datetime"] = datetime_to_str(datetime)
+            asset.fields["datetime"] = datetime_to_str(datetime)
 
     def get_assets(self) -> Dict[str, Asset]:
         """Get this item's assets.
diff --git a/tests/extensions/test_custom.py b/tests/extensions/test_custom.py
index 1bd723471..1e5ff1718 100644
--- a/tests/extensions/test_custom.py
+++ b/tests/extensions/test_custom.py
@@ -94,7 +94,7 @@ def __init__(self, item: pystac.Item) -> None:
 class AssetCustomExtension(CustomExtension[pystac.Asset]):
     def __init__(self, asset: pystac.Asset) -> None:
         self.catalog = asset
-        self.properties = asset.extra_fields
+        self.properties = asset.fields
         if asset.owner:
             if isinstance(asset.owner, pystac.Item):
                 self.additional_read_properties = [asset.owner.properties]
diff --git a/tests/extensions/test_pointcloud.py b/tests/extensions/test_pointcloud.py
index 7f21a7cf8..a3e5c78a5 100644
--- a/tests/extensions/test_pointcloud.py
+++ b/tests/extensions/test_pointcloud.py
@@ -271,7 +271,7 @@ def test_asset_extension(self) -> None:
         pc_item.add_asset("data", asset)
         ext = AssetPointcloudExtension(asset)
         self.assertEqual(ext.asset_href, asset.href)
-        self.assertEqual(ext.properties, asset.extra_fields)
+        self.assertEqual(ext.properties, asset.fields)
         self.assertEqual(ext.additional_read_properties, [pc_item.properties])
 
     def test_ext(self) -> None:

From c0edaffc2ca4a309626b55f4c1e545a346db5475 Mon Sep 17 00:00:00 2001
From: Jon Duckworth <duckontheweb@gmail.com>
Date: Tue, 31 Aug 2021 12:51:44 -0400
Subject: [PATCH 2/2] Fix some typing issues

---
 pystac/asset.py | 10 +++++-----
 pystac/base.py  | 17 +++++------------
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/pystac/asset.py b/pystac/asset.py
index 6872179ec..6d5205827 100644
--- a/pystac/asset.py
+++ b/pystac/asset.py
@@ -60,7 +60,7 @@ def __init__(
     @property
     def href(self) -> str:
         """Link to the asset object. Relative and absolute links are both allowed."""
-        return self._get_field("href", str, required=True)
+        return utils.get_required(self.fields.get("href"), self, "href")
 
     @href.setter
     def href(self, v: str) -> None:
@@ -69,7 +69,7 @@ def href(self, v: str) -> None:
     @property
     def title(self) -> Optional[str]:
         """Optional displayed title for clients and users."""
-        return self._get_field("title", str)
+        return self.fields.get("title")
 
     @title.setter
     def title(self, v: Optional[str]) -> None:
@@ -80,7 +80,7 @@ def description(self) -> Optional[str]:
         """A description of the Asset providing additional details, such as how it was
         processed or created. CommonMark 0.29 syntax MAY be used for rich text
         representation."""
-        return self._get_field("description", str)
+        return self.fields.get("description")
 
     @description.setter
     def description(self, v: Optional[str]) -> None:
@@ -90,7 +90,7 @@ def description(self, v: Optional[str]) -> None:
     def media_type(self) -> Optional[Union[MediaType, str]]:
         """Optional description of the media type. Registered Media Types are preferred.
         See :class:`~pystac.MediaType` for common media types."""
-        return self._get_field("type", str)
+        return self.fields.get("type")
 
     @media_type.setter
     def media_type(self, v: Optional[Union[MediaType, str]]) -> None:
@@ -100,7 +100,7 @@ def media_type(self, v: Optional[Union[MediaType, str]]) -> None:
     def roles(self) -> Optional[List[str]]:
         """Optional, Semantic roles (i.e. thumbnail, overview, data, metadata) of the
         asset."""
-        return self._get_field("roles", List[str])
+        return self.fields.get("roles")
 
     @roles.setter
     def roles(self, v: Optional[List[str]]) -> None:
diff --git a/pystac/base.py b/pystac/base.py
index 152edfbe1..f104cf114 100644
--- a/pystac/base.py
+++ b/pystac/base.py
@@ -1,7 +1,5 @@
 from copy import deepcopy
-from typing import Any, Dict, Type, TypeVar
-
-from pystac.utils import get_required
+from typing import Any, Dict, TypeVar
 
 
 V = TypeVar("V")
@@ -9,25 +7,20 @@
 
 class JSONObject:
     """A base class for objects that can be represented as JSON objects.
-    
+
     Instances of ``JSONObject`` have a :attr:`JSONObject.fields` attribute that is a
     dictionary representation of the JSON object. Any property getters/setters on
     inheriting classes MUST be sure to keep this dictionary up-to-date.
     """
-    fields = {}
+
+    fields: Dict[str, Any] = {}
 
     def to_dict(self, preserve_dict: bool = True) -> Dict[str, Any]:
         if preserve_dict:
             return deepcopy(self.fields)
         else:
             return self.fields
-    
-    def _get_field(self, name: str, _type: Type, *, required: bool = False) -> V:
-        if required:
-            return get_required(self.fields.get(name), self, name)
-        else:
-            return self.fields.get(name)
-    
+
     def _set_field(self, name: str, v: Any, *, pop_if_none: bool = False) -> None:
         if v is None and pop_if_none:
             self.fields.pop(name, None)