Future of this package? #576
Comments
|
Thank you for expressing your interests and concerns about the direction of JADNC.
In general, the goal of the framework is to become more production ready. For v4 this primarily means
At the same we are trying to keep the codebase simple and maintainable because of the very limited development resources we have available. We have insufficient capacity to fully support every broken/incomplete feature that is currently in v3.1 and as such we are forced to make a selection. We try to prioritise those features that are crucial to the json:api spec and the production-readiness of the framework, and as a result we will need to drop support for certain other features.
The decoupling of the framework through the removal of In practice, this violation leads to very poor testability of the framework as well as any of app that is built with it. This is clearly reflected in the test coverage: for the majority of the core functionality there are no unit/integration tests but only end-to-end tests. This is due to the inability to properly isolate elements like The removal of
In short: the feature is far from being production ready because it is only partially implemented and barely tested. To have a stable core API, we should either fully support it or (at least for now) remove it. See #553 for details. We consider this feature to be less mission critical for JADNC and because we have limited resources available we will not be able to maintain it. As such we are in favour of removing such complexity for now, but nevertheless we are very open to efforts to reintroducing it.
We acknowledge that bulk operations is a very useful feature as it is very close to the core spec. We plan to reintroduce it in one of the first releases after v4.0.0. Hopefully by this time the official extension will have been released so that we can ensure its stability. We are even considering to bring it back it in v4.0.0, but as of right now we do not have the development resources available to reintegrate it in the refactored framework immediately. Any efforts to reintegrate it are extremely welcome.
We have not removed client generated IDs: this is a core part of the spec. We also have not removed nested sparse field support: in fact, in v4 the logic related to this has been centralized in a dedicated service allowing for better testability and extensibility. See this internal document for details.
Note that a significant part of that number is caused to changes in test/wiki files which wont affect a migration to v4. Nevertheless, we are aware that v4 brings along a substantial amount of non-gradual breaking changes and that a migration will be tedious. Ideally we would introduce the decoupling of the framework through a phased implementation, but with the extent to which the framework is currently tightly coupled right now such strategy would take a long time to be fulfilled and produce a lot of overhead. In the context of our limited resources we are in favour of avoiding such overhead and introducing the breaking changes all at once with a very detailed migration guide. We would then immediately arrive at having a stable core API for which we would offer long term support.
|
|
Awesome response and FWIW I think the future direction of this project is
very promising.
…On Wed, Oct 16, 2019 at 11:17 AM Maurits Moeys ***@***.***> wrote:
Thank you for expressing your interests and concerns about the direction
of JADNC.
I would like to ask about future of this package [...] seems that this
package completely lost it's original direction and purpose [...] Are there
any reasons for these changes?
In general, the goal of the framework is to become more production ready.
For v4 this primarily means
1. to introduce proper testability of apps that are built on JADNC
(apps that are not testable are not production ready, also see #251
<#251>)
2. to be able to implement authorization without having to entirely
override/rewrite the service layer (the scope of JADNC would be very
limited if it is too complicated to implement such a basic thing, see
#552 <#552>,
#547 <#547>,
#473 <#473>)
3. to stabilize the core API (i.e. fixing violations of the json:api
spec and fixing/removing broken features in general).
At the same we are trying to keep the codebase simple and maintainable
because of the very limited development resources we have available. We
have insufficient capacity to fully support every broken/incomplete feature
that is currently in v3.1 and as such we are forced to make a selection. We
try to prioritise those features that are crucial to the json:api spec and
the production-readiness of the framework, and as a result we will need to
drop support for certain other features.
Like with removal of JsonApiContext which was replaced with four or more
different services? What purpose does this change have?
The decoupling of the framework through the removal of JsonApiContext is
a crucial step towards improved testability (and therefore
production-readiness).JsonApiContext contains a high amount of strongly
diverging responsibilities and it is injected in almost every service
throughout the framework. This is a serious problem because it strongly
violates Single Responsibility: it is a god object
<https://sourcemaking.com/antipatterns/the-blob> anti-pattern.
In practice, this violation leads to very poor testability of the
framework as well as any app that is built with it. This is clearly
reflected in the test coverage: for the majority of the core functionality
there are no unit/integration tests but only end-to-end tests. This is due
to the inability to properly isolate elements like DefaultEntityRepository
or EntityResourceService for unit testing, which is a direct result of
various layers of the framework sharing responsibilities they should not be
sharing and them being tightly coupled through omni-services like
JsonApiContext.
The removal of JsonApiContext was first proposed in #253
<#253>.
Strongly related to this is the decoupling of the serialization layer, see
this
<https://github.com/json-api-dotnet/JsonApiDotNetCore/blob/dev-v4/wiki/v4/content/serialization.md>
internal document.
For example why did You removed ResourceEntitySeparation and mapping logic
In short: the feature is far from being production ready because it is
only partially implemented and barely tested. To have a stable core API, we
should either fully support it or (at least for now) remove it. See #553
<#553> for
details. We consider this feature to be less mission critical for JADNC and
because we have limited resources available we will not be able to maintain
it. As such we are in favour of removing such complexity for now, but
nevertheless we are very open to efforts to reintroducing it.
or Bulk operations?
We acknowledge that bulk operations is a very useful feature as it is very
close to the core spec. We plan to reintroduce it in one of the first
releases after v4.0.0. Hopefully by this time the official extension will
have been released so that we can ensure its stability. We are even
considering to bring it back it in v4.0.0, but as of right now we do not
have the development resources available to reintegrate it in the
refactored framework immediately. Any efforts to reintegrate it are
extremely welcome.
Removed possibility to control if client can generate IDs. Dropped support
for nested sparse fields
We have not removed client generated IDs: this is a core part of the spec.
We also have not removed nested sparse field support: in fact, in v4 the
logic related to this has been centralized in a dedicated service allowing
for better testability and extensibility. See this
<https://github.com/json-api-dotnet/JsonApiDotNetCore/blob/dev-v4/wiki/v4/content/query-parameter-services.md>
internal document for details.
Current difference between v3 and v4 is more than 400 changed files. With
current progress You are making it almost impossible to migrate from v3 to
v4.
Note that a significant part of that number is caused to changes in
test/wiki files which wont affect a migration to v4. Nevertheless, we are
aware that v4 brings along a substantial amount of non-gradual breaking
changes and that a migration will be tedious.
Ideally we would introduce the decoupling of the framework through a
phased implementation, but with the extent to which the framework is
currently tightly coupled right now such strategy would take a long time to
be fulfilled and produce a lot of overhead. In the context of our limited
resources we are in favour of avoiding such overhead and introducing the
breaking changes all at once with a very detailed migration guide. We would
then immediately arrive at having a stable core API for which we would
offer long term support.
As a final note, we appreciate that you're expressing your concerns and
are open to discuss any others you might have.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#576?email_source=notifications&email_token=AABLPDDNATUWVKXTMMEYJEDQO3S4XA5CNFSM4JA7EVZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBL6V6Y#issuecomment-542632699>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABLPDHUJM5NXYRLL5VSBLTQO3S4XANCNFSM4JA7EVZA>
.
|
|
Thank you for reply. Don't get me wrong I always praise every attempt to provide support or growth to publicly accessible code. I just think that these massive changes in short time without backward compatibility could just hurt community of developer using this framework. I can understand that sometimes it's crucial for future to change even basic things. Improved testability, simplification for some implementations or simplification for wider audience can be a reason for that. But these things can be done and should be done with much more secure approach. Like making JsonApiContext deprecated and introduce new approach and give developers time to adapt to these changes. Just to say "We are deleting it right now" and let other developers deal with this is selfish attitude. I am sorry but reason like "because of the very limited development resources we have available" are just poor excuse. Removal of ResourceEntitySeparation because "In short: the feature is far from being production ready because it is only partially implemented and barely tested or we have limited resources available We will not be able to maintain it" is from my point of view another excuse. It's already there so when You remove it You're just messing around with stability of other projects. The same thing is with bulk operations. You are removing features which could be already implemented in some projects. You should approach it as if it is implemented everywhere. With this You are telling to people we don't think this feature is no longer needed just deal with it. No matter how much your project depends on it. I overlooked changes about client generated IDs. Finally to the final problem of these changes. As I mentioned above. To make these changes without ensuring atleast some backward compatibility even with good intent behind them is selfilsh towards one use-case. You are pushing your problem with limited resources to other developers. They will need to rewrite a big part of their logic and broke some other features or even due to migration to newer version just block their own development until migration will be completely finished and everything adapted. In the worst case scenario until some of the removed features are again implemented you are forcing users to end up with version 3.1. If I understand this correctly then you or your company took ownerhip of this framework and are investing your resources to the development of this package? |
|
I agree that some level of backward compatibility should be maintained, and we will (probably) do so in the form of a manual for people upgrading, to make upgrading an as seamless as possible experience. Because of the introduction of hooks some time ago we think that 95% of the use cases of users will be covered by the aforementioned hooks. I think this will in many ways lessen the 'pain' of migration. Before I go into the points you posit: we have limited resources, as does a Microsoft, Apple, etc. If we didnt devote any resources this project, v3.1 would still be at the same numbering. We have done bug fixes in the v3 branch until we thought we needed v4. The original author does not have the time or resources available anymore. And he (@jaredcnance) should not be forced to work on something unless it has his interest or he has time left. It is open source, not work source. As for your points.
You can stay on v3, there is nothing keeping you from upgrading, but if you want added functionality, you will have to migrate. Such is life. The needed changes for making hooks available, and making sure that we adhere to a single-responsibility for classes is what made way for v4. Also, you are more than welcome than make sure that batch updates are in v4.
Why? We are pushing out V4, not v3.3. If you have All in all, I think the main takeaway here is: if you value the backwards compatibility as much as you do; you are more than welcome to creating a pullrequest, commenting on issues, and/or contributing in any way shape or form. |
|
In principle you are completely right @House-MD about everything. In theory everything should be done to maintain backward compatibility, but in practice however, there is always the consideration of how to distribute resources. A key consideration here is that the majority of the features that have been removed/altered were experimental and not documented (some not even fully functioning), and because of this, and in general because of the scarce testability of the framework, we believe the adoption of the framework is still pretty low. As such in this stage of the framework there should be relatively little demand for backward compatibility compared to bringing the framework to a production ready state. Nevertheless, it is possible that we're strongly underestimating the backward compatibility issue. Still, in this case, it would not make sense to introduce backward compatibility to v4.0.0 because
However we could decide to release a v3.3 that marks all the to-be-removed methods as obsolete and introduces those parts of the new API that are backward-compatible. I will open up a tracking issue to monitor the demand for this. If it turns out that a lot of users want it, we will do it. Please leave your thumbs-up there to cast your vote |
|
Any remaining comments can be expressed here #585 |
I would like to ask about future of this package. As I see from all the changes to the code and tests, it seems that this package completely lost it's original direction and purpose which was to be framework for easier implementition of JSON:API standard. Are there any reasons for these changes?
It seems that this package became a complete mess since version 3.1.0 and absolutely incompatible. Like with removal of JsonApiContext which was replaced with four or more different services? What purpose does this change have? And this is only beginning. For example why did You removed ResourceEntitySeparation and mapping logic or Bulk operations? Removed possibility to control if client can generate IDs. Dropped support for nested sparse fields because they can be Attributes and also Relationships.
Current difference between v3 and v4 is more than 400 changed files. With current progress You are making it almost impossible to migrate from v3 to v4.
The text was updated successfully, but these errors were encountered: