Re: Sling API exporting @ConsumerType class
Hi, I had a lengthy offline discussion with Carsten regarding this topic, and I did not really find a good solution, without the need to deal with this tight coupling. Also in order not to overly complicate things I decided to undo SLING-12062 and then close the ticket as incomplete. The small benefit of it does not justify the handling of the coupling. Jörg
Re: Sling API exporting @ConsumerType class
Are we sure that this is the right solution? With the proposed solution, scripting core needs to be changed regardless. But more important, now every bundle that uses LazyBindings will get a narrow package range not just scripting.core. Or in other words, scripting.core will continue to have a narrow package range and you will run into the same problem with the next addition to LazyBindings. Regards Carsten Are we sure that no one out there is using LazyBindings in the same way as On 13.06.2024 21:43, Jörg Hoh wrote: Hi Julian, regarding the "LazyBindings.putOnly" method, the motivation was a mostly about performance, as I found a number of occurrences, where the put method actually resolved the supplier, but the return value of put() was not used at all. Also I want to avoid any change on the semantics of put, because for that I would need a major version bump; besides that the API contract of java.script.Bindings says that null is only returned if there was no value previously associated with that name. For that reason I believe that it's best if * the ProtectedBindings moves into the SLING API bundle (package org.apache.sling.api.scripting, next to the LazyBindings) * and the both LazyBindings and ProtectedBindings are marked as ProviderType to make that take effect I need to release a new version of scirpting.core as well, but then the narrow import range for the package org.apache.sling.api.scripting is more relaxed, and allows an easier evolution of the API. Jörg Am Mi., 12. Juni 2024 um 14:22 Uhr schrieb Julian Sedding < jsedd...@gmail.com>: Hi Jörg In general, I agree that we could, and maybe should, evaluate which types in the Sling API should be @ProviderType instead of @ConsumerType. My assumption would be that most types should be @ProviderType. I don't know what sort of versioning change adding the @ProviderType annotation causes, however. Regarding your specific case, I wonder if it would be acceptable to remove the "putOnly" method, and instead modify the "put" method to return "null" for in the case where a key was inserted, but the supplier never evaluated. Of course that would change the behaviour, so might cause issues in some edge cases. Regards Julian On Mon, Jun 10, 2024 at 12:38 PM Jörg Hoh wrote: Eric pointed out correctly, that there was tight connection between SLING API and Scripting core, as the package import range on scripting core (bundle version 2.4.8) is quite narrow for the package org.apache.sling.api.scripting: org.apache.sling.api.scripting; version="[2.5, 2.6)" This is caused by the class LazyBindings being a ConsumerType ( https://github.com/apache/sling-org-apache-sling-api/blob/bdbd1768969917d0e2436d5b008fff80aaa729dc/src/main/java/org/apache/sling/api/scripting/LazyBindings.java#L56 ) and the scripting core inheriting from it (in ProtectedBindings). My latest extension to it (SLING-12062) then caused a bump of the exported version, thus breaking the existing imports. Of course I could do a release of the Scripting Core as well and make sure that it works with the package version as well, but I wonder if this is the right thing to do. Should the Sling API export ConsumerType interfaces and classes at all, which can cause this type of problems, or should we try to avoid them? This is a more general question; and in case that we want to avoid those, the question is still how we want to move forward with this specific instance of it. My personal opinion is * to avoid @ConsumerType exports in the Sling API in general and * move the ProtectedBindings class, which inherits from the LazyBindings over to the API that would allow us to have a wider import range on the scripting.core side. WDYT? Jörg -- https://cqdump.joerghoh.de -- Carsten Ziegeler Adobe cziege...@apache.org
Re: Sling API exporting @ConsumerType class
Hi Julian, regarding the "LazyBindings.putOnly" method, the motivation was a mostly about performance, as I found a number of occurrences, where the put method actually resolved the supplier, but the return value of put() was not used at all. Also I want to avoid any change on the semantics of put, because for that I would need a major version bump; besides that the API contract of java.script.Bindings says that null is only returned if there was no value previously associated with that name. For that reason I believe that it's best if * the ProtectedBindings moves into the SLING API bundle (package org.apache.sling.api.scripting, next to the LazyBindings) * and the both LazyBindings and ProtectedBindings are marked as ProviderType to make that take effect I need to release a new version of scirpting.core as well, but then the narrow import range for the package org.apache.sling.api.scripting is more relaxed, and allows an easier evolution of the API. Jörg Am Mi., 12. Juni 2024 um 14:22 Uhr schrieb Julian Sedding < jsedd...@gmail.com>: > Hi Jörg > > In general, I agree that we could, and maybe should, evaluate which > types in the Sling API should be @ProviderType instead of > @ConsumerType. My assumption would be that most types should be > @ProviderType. I don't know what sort of versioning change adding the > @ProviderType annotation causes, however. > > Regarding your specific case, I wonder if it would be acceptable to > remove the "putOnly" method, and instead modify the "put" method to > return "null" for in the case where a key was inserted, but the > supplier never evaluated. Of course that would change the behaviour, > so might cause issues in some edge cases. > > Regards > Julian > > On Mon, Jun 10, 2024 at 12:38 PM Jörg Hoh > wrote: > > > > Eric pointed out correctly, that there was tight connection between SLING > > API and Scripting core, as the package import range on scripting core > > (bundle version 2.4.8) is quite narrow for the package > > org.apache.sling.api.scripting: > > > > org.apache.sling.api.scripting; version="[2.5, 2.6)" > > > > This is caused by the class LazyBindings being a ConsumerType ( > > > https://github.com/apache/sling-org-apache-sling-api/blob/bdbd1768969917d0e2436d5b008fff80aaa729dc/src/main/java/org/apache/sling/api/scripting/LazyBindings.java#L56 > ) > > and the scripting core inheriting from it (in ProtectedBindings). > > > > My latest extension to it (SLING-12062) then caused a bump of the > exported > > version, thus breaking the existing imports. > > > > Of course I could do a release of the Scripting Core as well and make > sure > > that it works with the package version as well, but I wonder if this is > the > > right thing to do. > > > > Should the Sling API export ConsumerType interfaces and classes at all, > > which can cause this type of problems, or should we try to avoid them? > This > > is a more general question; and in case that we want to avoid those, the > > question is still how we want to move forward with this specific instance > > of it. > > > > My personal opinion is > > * to avoid @ConsumerType exports in the Sling API in general and > > * move the ProtectedBindings class, which inherits from the LazyBindings > > over to the API > > > > that would allow us to have a wider import range on the scripting.core > side. > > > > WDYT? > > > > Jörg > > > > > > -- > > https://cqdump.joerghoh.de > -- https://cqdump.joerghoh.de
Re: Sling API exporting @ConsumerType class
Hi, The following example table lists the ranges being generated for the exported version 1.1.0 depending on the usage and annotations: Annotation on class/interface | Version | Implemented/Extended | Import-Package Version Range | Versioning Policy @ConsumerType | 1.1.0 | yes | [1.1, 2.0) | Consumer @ConsumerType | 1.1.0 | no | [1.1, 2.0) | Consumer @ProviderType | 1.1.0 | yes | [1.1, 1.2) | Provider @ProviderType | 1.1.0 | no | [1.1, 2.0) | Consumer So in general, backwards incompatible changes on provider types are less likely to have an impact (because they only affect providers with narrow import ranges) but still may break bundles. Konrad > On 12. Jun 2024, at 14:21, Julian Sedding wrote: > > Hi Jörg > > In general, I agree that we could, and maybe should, evaluate which > types in the Sling API should be @ProviderType instead of > @ConsumerType. My assumption would be that most types should be > @ProviderType. I don't know what sort of versioning change adding the > @ProviderType annotation causes, however. > > Regarding your specific case, I wonder if it would be acceptable to > remove the "putOnly" method, and instead modify the "put" method to > return "null" for in the case where a key was inserted, but the > supplier never evaluated. Of course that would change the behaviour, > so might cause issues in some edge cases. > > Regards > Julian > > On Mon, Jun 10, 2024 at 12:38 PM Jörg Hoh > wrote: >> >> Eric pointed out correctly, that there was tight connection between SLING >> API and Scripting core, as the package import range on scripting core >> (bundle version 2.4.8) is quite narrow for the package >> org.apache.sling.api.scripting: >> >> org.apache.sling.api.scripting; version="[2.5, 2.6)" >> >> This is caused by the class LazyBindings being a ConsumerType ( >> https://github.com/apache/sling-org-apache-sling-api/blob/bdbd1768969917d0e2436d5b008fff80aaa729dc/src/main/java/org/apache/sling/api/scripting/LazyBindings.java#L56) >> and the scripting core inheriting from it (in ProtectedBindings). >> >> My latest extension to it (SLING-12062) then caused a bump of the exported >> version, thus breaking the existing imports. >> >> Of course I could do a release of the Scripting Core as well and make sure >> that it works with the package version as well, but I wonder if this is the >> right thing to do. >> >> Should the Sling API export ConsumerType interfaces and classes at all, >> which can cause this type of problems, or should we try to avoid them? This >> is a more general question; and in case that we want to avoid those, the >> question is still how we want to move forward with this specific instance >> of it. >> >> My personal opinion is >> * to avoid @ConsumerType exports in the Sling API in general and >> * move the ProtectedBindings class, which inherits from the LazyBindings >> over to the API >> >> that would allow us to have a wider import range on the scripting.core side. >> >> WDYT? >> >> Jörg >> >> >> -- >> https://cqdump.joerghoh.de
Re: Sling API exporting @ConsumerType class
Hi Jörg In general, I agree that we could, and maybe should, evaluate which types in the Sling API should be @ProviderType instead of @ConsumerType. My assumption would be that most types should be @ProviderType. I don't know what sort of versioning change adding the @ProviderType annotation causes, however. Regarding your specific case, I wonder if it would be acceptable to remove the "putOnly" method, and instead modify the "put" method to return "null" for in the case where a key was inserted, but the supplier never evaluated. Of course that would change the behaviour, so might cause issues in some edge cases. Regards Julian On Mon, Jun 10, 2024 at 12:38 PM Jörg Hoh wrote: > > Eric pointed out correctly, that there was tight connection between SLING > API and Scripting core, as the package import range on scripting core > (bundle version 2.4.8) is quite narrow for the package > org.apache.sling.api.scripting: > > org.apache.sling.api.scripting; version="[2.5, 2.6)" > > This is caused by the class LazyBindings being a ConsumerType ( > https://github.com/apache/sling-org-apache-sling-api/blob/bdbd1768969917d0e2436d5b008fff80aaa729dc/src/main/java/org/apache/sling/api/scripting/LazyBindings.java#L56) > and the scripting core inheriting from it (in ProtectedBindings). > > My latest extension to it (SLING-12062) then caused a bump of the exported > version, thus breaking the existing imports. > > Of course I could do a release of the Scripting Core as well and make sure > that it works with the package version as well, but I wonder if this is the > right thing to do. > > Should the Sling API export ConsumerType interfaces and classes at all, > which can cause this type of problems, or should we try to avoid them? This > is a more general question; and in case that we want to avoid those, the > question is still how we want to move forward with this specific instance > of it. > > My personal opinion is > * to avoid @ConsumerType exports in the Sling API in general and > * move the ProtectedBindings class, which inherits from the LazyBindings > over to the API > > that would allow us to have a wider import range on the scripting.core side. > > WDYT? > > Jörg > > > -- > https://cqdump.joerghoh.de
Sling API exporting @ConsumerType class
Eric pointed out correctly, that there was tight connection between SLING API and Scripting core, as the package import range on scripting core (bundle version 2.4.8) is quite narrow for the package org.apache.sling.api.scripting: org.apache.sling.api.scripting; version="[2.5, 2.6)" This is caused by the class LazyBindings being a ConsumerType ( https://github.com/apache/sling-org-apache-sling-api/blob/bdbd1768969917d0e2436d5b008fff80aaa729dc/src/main/java/org/apache/sling/api/scripting/LazyBindings.java#L56) and the scripting core inheriting from it (in ProtectedBindings). My latest extension to it (SLING-12062) then caused a bump of the exported version, thus breaking the existing imports. Of course I could do a release of the Scripting Core as well and make sure that it works with the package version as well, but I wonder if this is the right thing to do. Should the Sling API export ConsumerType interfaces and classes at all, which can cause this type of problems, or should we try to avoid them? This is a more general question; and in case that we want to avoid those, the question is still how we want to move forward with this specific instance of it. My personal opinion is * to avoid @ConsumerType exports in the Sling API in general and * move the ProtectedBindings class, which inherits from the LazyBindings over to the API that would allow us to have a wider import range on the scripting.core side. WDYT? Jörg -- https://cqdump.joerghoh.de
