♻️ Datcore-adapter refactoring #7270
Conversation
sanderegg
requested review from
pcrespov,
matusdrobuliak66 and
giancarloromeo
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7270 +/- ##
==========================================
+ Coverage 85.25% 85.31% +0.06%
==========================================
Files 1684 1674 -10
Lines 65237 65055 -182
Branches 1106 1106
==========================================
- Hits 55618 55502 -116
+ Misses 9305 9239 -66
Partials 314 314
Continue to review full report in Codecov by Sentry.
|
sanderegg
force-pushed
the
storage/datcore-adapter-refactoring
branch
from
b1ccf52 to
2521650
Compare
|
packages/models-library/src/models_library/api_schemas_datcore_adapter/datasets.py
Show resolved
Hide resolved
packages/models-library/src/models_library/api_schemas_datcore_adapter/datasets.py
Show resolved
Hide resolved
| created_at: Annotated[datetime.datetime, Field(alias="createdAt")] | ||
| updated_at: Annotated[datetime.datetime, Field(alias="updatedAt")] | ||
|
|
||
| def to_api_model(self) -> PackageMetaData: |
There was a problem hiding this comment.
We started removing this dependency such that domain models know nothing about schemas, but rather the other way around.
Check from_domain_model and to_domain_model in schemas in https://github.com/itisfoundation/osparc-simcore/blob/a9c4d5450d916851086772705ba9d6c8883b0240/packages/models-library/src/models_library/api_schemas_webserver/groups.py
I also curated some details and an example with chatty for convenience that adapts to clarify :
Both Domain Models and Schemas are implemented with Pydantic, but they serve different purposes and should be kept separate.
| Aspect | Domain Model (Service Layer) | Schema (API Layer) |
|---|---|---|
| Purpose | Represents business entities | Validates API request/response data |
| Used In | Service Layer (services/) | Controller Layer (schemas/) |
| Should Contain? | Core business fields & logic | API-specific fields (e.g., validation rules, optional fields) |
| Why Separate? | Keeps business logic independent of transport layer (REST, RPC) | Ensures API input/output correctness/backwards compatibility and decoupling |
Example
books/
├── books_rest.py # REST Controller
├── books_service.py # Service Layer
├── books_repository.py # Repository Layer (Database Access)
├── books_models.py # Database Models (SQLAlchemy)
├── books_domain.py # Domain Models (Business Entities)
├── books_schemas.py # API Request/Response Schemas (with adapter method)
1️⃣ Domain Model (Service Layer)
📌 Stored in books_domain.py
from pydantic import BaseModel
class BookModel(BaseModel):
"""Represents a Book entity inside the business logic."""
id: int
title: str
author: str
year: int✅ Used in the Service Layer
✅ Independent of database and API schemas
2️⃣ Schema (API Layer with from_domain_model Method)
📌 Stored in books_schemas.py
from pydantic import BaseModel, Field
from books_domain import BookModel
class BookCreate(InputSchema):
"""Validates incoming API request to create a book."""
title: str = Field(..., min_length=3, max_length=255)
author: str = Field(..., min_length=3, max_length=100)
year: int = Field(..., ge=1000, le=2100)
def to_domain_model(self) -> BookModel:
# ...
class BookResponse(OutputSchema):
"""Defines the API response format."""
id: int
title: str
author: str
year: int
@classmethod
def from_domain_model(cls, book: BookModel) -> Self:
"""Converts a domain model to an API response schema."""
return cls(**book.dict())✅ Encapsulates conversion logic inside BookResponse
✅ Keeps the transformation method tied to the API Schema
✅ Avoids scattered adapter functions
3️⃣ Service Layer Uses Domain Model
📌 Stored in books_service.py
from books_domain import BookModel
from books_repository import BookRepository
class BookService:
def __init__(self):
self.repository = BookRepository()
async def create_book(self, book_data: dict) -> BookModel:
"""Handles business logic before saving a book."""
book_id = await self.repository.save_book(book_data)
return BookModel(id=book_id, **book_data) # ✅ Returns a Domain Model✅ Service layer returns domain models (not API schemas)
4️⃣ Controller Uses Schema’s from_domain_model() Method
📌 Stored in books_rest.py
from fastapi import APIRouter, HTTPException
from books_service import BookService
from books_schemas import BookCreate, BookResponse
router = APIRouter()
book_service = BookService()
@router.post("/books/", response_model=BookResponse)
async def create_book(book: BookCreate):
"""Handles book creation request."""
created_book = await book_service.create_book(book.to_domain_model())
return BookResponse.from_domain_model(created_book) # ✅ Uses schema method✅ Uses BookCreate for input validation
✅ Uses book.to_domain_model() for conversion
✅ Uses BookResponse.from_domain_model() for conversion
✅ Makes transformation an intuitive part of the schema
There was a problem hiding this comment.
"We started removing this dependency such that domain models know nothing about schemas, but rather the other way around."
so your api models (living in models-library) know about the models inside the service? that does not sound right at all. are you sure?
There was a problem hiding this comment.
I agree, as discussed already many times this the way to go. :)
There was a problem hiding this comment.
I prefer having a separate utils model in that case
There was a problem hiding this comment.
@pcrespov that means all your models are living in models-library? doesn't seem right.
There was a problem hiding this comment.
"We started removing this dependency such that domain models know nothing about schemas, but rather the other way around." so your api models (living in models-library) know about the models inside the service? that does not sound right at all. are you sure?
@pcrespov that means all your models are living in models-library? doesn't seem right.
No, that is not the case.
@sanderegg let me elaborate a bit more
The models_library has
- schemas (under
api_schema_*) - domain models
Then
-
- there, domain models can be imported in schemas but not the other way around.
-
- but, when you have a domain model that lives in your service (e.g.
models/domains) then you create the adaptersfrom_domain_model/to_domain_modelas free function in your own service. There is no need to move it tomodels_library
- but, when you have a domain model that lives in your service (e.g.
The point 2 does not happen a lot in the webserver because most of the domain models are already in the models_library (that is why i did not mention before)
An alternative would be that all adapters live in a separate place i.e.
books/
├── books_rest.py # REST Controller
├── books_service.py # Service Layer
├── books_repository.py # Repository Layer (Database Access)
├── books_models.py # Database Models (SQLAlchemy)
├── books_domain.py # Domain Models (Business Entities)
├── books_schemas.py # API Request/Response Schemas
├── books_adapter.py # 📌 Converts Domain Model → API Schema
What do these changes do?
pathanddisplay_pathtoGET /packages/{package_id}/filesresponserequired by #7200
Related issue/s
How to test
Dev-ops checklist