Skip to content
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

feat!: Adapt Restify to JSONAPI standards #582

Open
wants to merge 42 commits into
base: 8.x
Choose a base branch
from

Conversation

maicol07
Copy link
Contributor

@maicol07 maicol07 commented Sep 4, 2023

Fixes #558
Will fix: #562

Progress:

⚒️ means "Currently in progress"

In this update, the serialization process for eager relationships in the Repository class was refined and expanded. The commit includes the addition of a new boolean flag '$isRelationship', representing whether the repository is serializing for a relationship listing.

Moreover, the 'map' method for collecting the related model ID was significantly enhanced with a 'groupBy' and 'flatmap' function chain to better handle collections. A new property called 'included' has also been introduced to handle serialized content not represented by 'MissingValue'.

Changes were also made to JSON serialisation, where a check for 'isRelationship' has been added to determine whether to run 'serializeForRelationship' or 'serializeForShow'.

The alterations aim to improve the handling and representation of data relationships in the application, ensuring a more accurate and efficient data display.
@what-the-diff
Copy link

what-the-diff bot commented Sep 4, 2023

PR Summary

  • Enhancement of RelatedCollection Class
    A new feature was added to filter out only the matchable items using a new onlyMatchable() method in RelatedCollection class.

  • Introduction of Matchable Attributes to EagerField
    A new property named $matchableAttributes was introduced into the EagerField class, equipped with corresponding methods to enable interaction, checking and retrieving matchable items.

  • Enhancement of Filter in MatchFilter Class
    The filter() method in the MatchFilter class can now accommodate an additional parameter $no_eager. The logic inside the method has been adjusted accordingly to incorporate this change.

  • Adjustment of Filter Method in MatchesCollection
    The $filters variable assignment received a minor modification in the filter() method using the dot() method.

  • Improvements in ValidatingTrait Class
    Updates were made to several methods involving a switch from using $k->attribute to $k->getAttribute() when establishing the array keys.

  • Expansion in InteractWithSearch Trait
    The collectMatches() method now also identifies related matchable fields from the RelatedCollection, broadening its capabilities.

  • Test Files Modifications
    Several changes were made to the test files to align with the aforementioned updates in the payload structure and in the JSON structure returned by the API. Assertions were also modified to check different array components. In addition, the test request payload structure received modifications in order to integrate the new data > attributes field. A few test methods have been updated accordingly to accommodate these changes. The affected classes include FieldActionTest, IndexRelatedFeatureTest, NestedRepositoryControllerTest, RepositoryIndexControllerTest, ProfileControllerTest, RepositoryShowControllerTest, ShowRelatedFeatureTest, and Prototypeable.

The `resolveIndexIncluded` function is updated to accept a Collection of items and perform a series of operations to serialize the data. This transformation maps each repository, flattens the result, filters to remove any instances of MissingValue, and ensures uniqueness based on id and type. This change is necessary to ensure the output conforms to the JSONAPI standards, enhancing interoperability with other applications.
The attributes returned from the method serializeForShow in the Repository were modified. In the previous version, various elements like 'id', 'type', 'attributes', 'relationships', and 'meta' were sent individually. These have now been grouped together under a new 'data' key. This change was implemented to improve the organization of the returned data and comply with JSON API Specification.
The previous method of using flatMap directly was causing issues when a nested collection was encountered. This update replaces flatMap with map and adds conditional checking with `when` for better handling of nested collections. Now, If a collection within the main collection is identified, it converts it into a flat sequence for proper processing."
The changes in the Repository file improve the handling of related items in the API response. By restructuring the serialization process, this commit ensures that even deeply nested relationships are correctly represented.

Instead of mapping over each related item and filtering the results, this update flatMaps over the related items, handling nested collections more explicitly and improving performance on large datasets. As a part of these changes, the show method now returns a JSON response.

Unnecessary flattening operations have been removed, simplifying the code and further improving efficiency. This change also ensures that item fields such as 'type' and 'id' that match the current repository are correctly excluded from the 'included' array.
Updated the serializeIncluded() method in Repository.php. Replaced the unique() function's parameters to handle an array instead of a nullable instance. Added a flatten(1) call before unique() to ensure uniqueness across all elements of the nested arrays, and sorted by 'type' and 'id'. This refactoring was needed to handle cases where repository data is in a nested array format instead of an object, and to ensure proper sorting of repositories.
@binaryk
Copy link
Collaborator

binaryk commented Sep 7, 2023

I think you can use the $eagerState property instead of isRelationship.

@maicol07
Copy link
Contributor Author

maicol07 commented Sep 7, 2023

I think you can use the $eagerState property instead of isRelationship.

The issue here is that $eagerState is used for the included (old relationships array). I need something else to identify the new slim relationships array and the included one. Maybe I can use an enum since Restify targets PHP 8.1+?

These changes consist of minor adjustments to the formatting of the Repository.php file. The adjustments are mainly to the way in which our arrow functions and other segments are formatted. These modifications were made to enhance the code's readability and maintain consistency throughout the file.
Refactored the merge operation in Repository.php to only process when nested_relationships is not empty. This change optimizes memory usage and performance by avoiding unnecessary operations for empty collections.
Modified the 'matches' method in InteractWithSearch Trait to improve readability and maintainability. Instead of using ternary operation, used if statement and array notation for better understanding. This change also makes it easy to modify the keyType in the future if necessary.
The changes in this commit firstly introduce the functionality of adding matchable attributes to the EagerField in LaravelRestify.
In Filters/MatchesCollection.php, the collection method from the request input has been modified to append on all keys and their inputs.
In Traits/InteractWithSearch.php, matchable fields from the repository are now added to the MatchesCollection.
The functions onlyMatchable() and matchable() have been added to RelatedCollection.php and EagerField.php to facilitate this new functionality.
Modified MatchFilter.php to accommodate the changes already mentioned. When there's a matchable field, now join statements have been added in the filter() method to make sure relevant records from related entity are included in the search.
Lastly, in EagerField.php, the new method getMatchables() has been introduced to return matchable attributes.
A change was made to add functionality to better handle "eager loading" in the MatchFilter class. Prior to this change, the code was using a join operation, which was found to produce empty results in related filter queries. To work around this issue, a subquery was used to circumvent the problematic join method. This ensures that any eager fetches of related data do not result in empty arrays, therefore offering more accurate inclusion and relationship responses.
Introduced "links" in the API response schema to meet JSON API specifications. This provides self-url and related-url in the API response, improving understandability and usability for consumers of the API. Removed a placeholder comment relating to this task.
The commit simplifies the handling of relationships in the repository by reducing redundancy and improving readability. Code related to the attachment of 'BelongsToMany' relations and association of 'BelongsTo' relations have been restructured. Also, it removes the comments that seemed to disable the section of the code handling relationships, primarily bringing back the condition check for the existence of relationships in the request data. This change will make the repository's code cleaner and more maintainable without altering its functionality.
The commit simplifies the handling of relationships in the repository by reducing redundancy and improving readability. Code related to the attachment of 'BelongsToMany' relations and association of 'BelongsTo' relations have been restructured. Also, it removes the comments that seemed to disable the section of the code handling relationships, primarily bringing back the condition check for the existence of relationships in the request data. This change will make the repository's code cleaner and more maintainable without altering its functionality.
…8.x-jsonapi-compatibility

# Conflicts:
#	src/Repositories/Repository.php
The related link url in the Repository class was hard-coded to use the '{repositoryId}'. This commit replaces it to dynamically fetch the id from the request object. This change ensures the related URL actually corresponds to the current repository instance, improving the coherence of our URL structure.
Removed unnecessary if block that checked for BelongsToMany relation. This check was creating redundancy, as the following repository instantiations and relationship handling were independent of the relation type. This change simplifies the code and improves performance by eliminating needless operations.
This commit addresses refactoring changes to improve the consistency of API response structure.
The change is to put 'attributes' inside 'data' key while sending or receiving data to or from API.
In original structure, data was directly accessed but now it is accessed through 'data.attributes', which is more descriptive and structured.
This approach is aligned with JSON:API specifications and improves readability and understanding of where the actual data lies. So, it is a refactor towards cleaner code and standardized API responses.
@maicol07
Copy link
Contributor Author

@binaryk I think the PR is ready to be reviewed. The only thing left is a documentation update to cover the new API format (I guess this can be labeled as Restify 9.0?)

There is also a test failing but I don't know how to fix it (I think I need your help for this one 😅)! Basically, it seems nested relationships aren't added to included. Related code:

https://github.com/maicol07/laravel-restify/blob/b0da4d86ab16b4ae9d05e118200d799e7213437b/src/Repositories/Repository.php#L542-L553

@maicol07 maicol07 marked this pull request as ready for review October 16, 2023 14:58
This commit refactors the assembly of 'meta' data in the Repository.php file. By utilizing ternary operators and compact()-function to assemble the 'meta' data, the code can avoid unnecessary creation of temporary arrays. This change improves readability and performance by reducing iteration over potentially large data structures.
@maicol07 maicol07 changed the title feat: Adapt Restify to JSONAPI standards feat!: Adapt Restify to JSONAPI standards Oct 27, 2023
The code modification handles a potential issue that might occur when the RestifyRequest does not contain any attributes. This patch ensures the "patch" function in our Repository class now defers to an empty array when no data attributes are given, preventing it from throwing an error in such cases.
@binaryk
Copy link
Collaborator

binaryk commented Dec 30, 2023

Thanks @maicol07 , can we make sure tests pass please?

@maicol07
Copy link
Contributor Author

Thanks @maicol07 , can we make sure tests pass please?

Yes, I want the last test to pass, however I tried to debug it but I can't understand why nested relationships aren't loaded in included. Am I missing something?

Copy link

netlify bot commented Mar 13, 2024

Deploy Preview for laravel-restify-docs canceled.

Name Link
🔨 Latest commit 3feefd1
🔍 Latest deploy log https://app.netlify.com/sites/laravel-restify-docs/deploys/65f191f1979b3800081247b1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Store/Update requests aren't working with JSONAPI payload format
2 participants