-
Couldn't load subscription status.
- Fork 4
DOCSP-54965: Java backend scaffolding setup #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One root .gitignore is considered to be a standard most repositories use. Is there a reason we want to have more than one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change does not seem to be related to java backend
|
|
||
| ## Technology Stack | ||
|
|
||
| - **Framework**: Spring Boot 3.2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spring Boot 3.2.x will be out of support in a couple of month - https://spring.io/projects/spring-boot#support
Should we consider taking 3.5.x?
| ## Technology Stack | ||
|
|
||
| - **Framework**: Spring Boot 3.2.0 | ||
| - **Java Version**: 17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Java 17 permissive licence support was ended in September 2024 - http://oracle.com/au/java/technologies/java-se-support-roadmap.html (see "End of Permissive Licensing of Java SE 17" paragraph).
I think we should consider Java 21 (I wouldn't jump to 25 yet, as it was just released).
|
|
||
| - **Framework**: Spring Boot 3.2.0 | ||
| - **Java Version**: 17 | ||
| - **MongoDB Driver**: MongoDB Java Driver 5.1.4 (Sync) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seem to me a bit old. I usually go with the latest. 5.6.1?
|
|
||
| This application provides a REST API for managing movie data from MongoDB's sample_mflix database. It demonstrates: | ||
|
|
||
| - Direct usage of the MongoDB Java Driver (not Spring Data MongoDB) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason why we do not want to use Spring Data for MongoDB? It will allow to avoid boiler plate code in such a simple app. You can still use mongoDB driver for more complex scenarios in the same app.
| InsertOneResult result = movieRepository.insertOne(movie); | ||
|
|
||
| if (!result.wasAcknowledged()) { | ||
| throw new RuntimeException("Movie insertion was not acknowledged by the database"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a good practice to use a custom exception instead of naked RuntimeException, especially when we already have some in the codebase.
| } | ||
|
|
||
| return movieRepository.findById(result.getInsertedId().asObjectId().getValue()) | ||
| .orElseThrow(() -> new RuntimeException("Failed to retrieve created movie")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we use ResourceNotFoundException?
| public InsertManyResult insertMany(List<Movie> movies) { | ||
| List<Document> documents = movies.stream() | ||
| .map(this::movieToDocument) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.collect(Collectors.toList()) -> toList()
| * Spring automatically manages the lifecycle and closes the client on shutdown. | ||
| */ | ||
| @Configuration | ||
| public class MongoConfig { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it will be extremely useful to have the setup for @transactional support as it is disabled by default in spring with mongoDB which I consider a huge spring data implementation gap.
It looks like the application code does not actually leverage transactions but any non-trivial app implementation will need this and it will be good if our sample has it as well.
I'm talking about this snippet of code from
https://github.com/johnlpage/MongoEnterpriseMicroserviceExamples/blob/main/memex/src/main/java/com/johnlpage/memex/config/MongoConfig.java
@Bean
public MongoTransactionManager transactionManager() {
LOG.info("MongoDB Native Transactions Enabled");
return new MongoTransactionManager(mongoDatabaseFactory);
}
More details if needed - https://docs.spring.io/spring-data/mongodb/reference/mongodb/client-session-transactions.html#mongo.transactions.reactive-tx-manager
| // MongoDB will automatically ignore this if the index already exists | ||
| moviesCollection.createIndex( | ||
| Indexes.compoundIndex( | ||
| Indexes.text("plot"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would create string constants in Movie.java class and use them here. Otherwise there is no clear way to document in Movie.java that there is coupling on the names of the fields of the class and there is no better way than full text search to find all such dependencies. If we introduce a constant it would allow IDE to show those dependencies in 'Find Usages' menu and hence make the dependency more transparent.
Of course this solution will look ugly if our code will have dependency on each and every field of the model (which is currently the case), however I'm going to suggest a solution to make it a bit cleaner as well.
| @Repository | ||
| public class MovieRepositoryImpl implements MovieRepository { | ||
|
|
||
| private final MongoCollection<Document> moviesCollection; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to make this MongoCollection moviesCollection and add pojo codec.
Then we will be able to get rid of all the boiler plate pojo conversion methods: movieToDocument, documentToMovie, awardsToDocument, documentToAwards, imdbToDocument, documentToImdb, tomatoesToDocument, documentToTomatoes, viewerToDocument, documentToViewer, criticToDocument, documentToCritic.
Please find the patch attached:
remove_pojo_conversion_boilerplate.patch
| * @param movie the Movie object | ||
| * @return the BSON Document | ||
| */ | ||
| private Document movieToDocument(Movie movie) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shared the patch in the comment above
| return new Document(field, order); | ||
| } | ||
|
|
||
| private boolean isUpdateRequestEmpty(UpdateMovieRequest request) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified to just 2 lines:
Map<String, Object> requestMap = objectMapper.convertValue(request, Map.class);
return requestMap.values().stream().allMatch(java.util.Objects::isNull);
Same with the next method. Please find the patch attached -
objectmapper.patch
Setup of Java Spring Boot backend for the MongoDB Sample MFlix application
This PR implements the initial setup of a Java Spring Boot backend for the MongoDB Sample MFlix application (parity with the current Express.js/TypeScript backend). The implementation demonstrates direct MongoDB Java Driver usage for educational purposes while following Spring Boot best practices.
What to Review
Database Configuration (
config/)DatabaseVerificationrobustDomain Models (
model/)DTOs (
model/dto/)Response Models (
model/response/)SuccessResponse<T>correct@JsonInclude(NON_NULL)behavior desiredRepository Layer (
repository/)Service Layer (
service/)Controller Layer (
controller/)Exception Handling (
exception/)What's NOT in This PR
Testing (Future Work)
Advanced API Endpoints (Future Work)
Documentation (Partial)
Build Verification
API Endpoints
All endpoints are fully implemented and match the current Express backend:
Query Parameters (GET /api/movies)
q- Full-text searchgenre- Filter by genre (case-insensitive)year- Filter by year (exact match)minRating- Minimum IMDB ratingmaxRating- Maximum IMDB ratinglimit- Results per page (default: 20, max: 100)skip- Number of results to skipsortBy- Field to sort by (default: title)sortOrder- Sort order: asc or desc (default: asc)Design Decisions
Why Lombok?
Why Custom Repository (Not Spring Data)?
Why Manual BSON Conversion?
Why Nested Classes?
Movie.Awardsvs separateMovieAwardsclass)Testing Recommendations
Manual Testing
./mvnw spring-boot:runcurl http://localhost:3001/curl http://localhost:3001/api/movies?limit=5curl http://localhost:3001/api/movies/{id}curl -X POST http://localhost:3001/api/movies -H "Content-Type: application/json" -d '{"title":"Test Movie"}'curl -X PUT http://localhost:3001/api/movies/{id} -H "Content-Type: application/json" -d '{"year":2024}'curl -X DELETE http://localhost:3001/api/movies/{id}