Product query extension is not executed for custom product model about aem-core-cif-components HOT 12 CLOSED

Hi @mhaack
You pointed me in the right direction and I've managed to find out what the problem was. In this tutorial they only set the custom model interface "MyProductTeaser" in the adaptable annotation. That was how I was doing it so it wasn't being picked up in the PageMetadataImpl.java. But it is now that I've included the core interface as well:

@Model(adaptables = SlingHttpServletRequest.class, adapters = { Product.class, com.adobe.cq.commerce.core.components.models.product.Product.class}, resourceType = ProductImpl.RESOURCE_TYPE )

By the way, I named my interface and implementation class as "Product" and "ProductImpl" as well which is where the confusion was. And I didn't need to make any extensions to the PageMetadataImpl.java class, I was just using the new interface I'd created to pass to the model factory. It would probably have made sense to name it slightly different.

It would probably be a good idea to update the tutorial so that the core Product model interface is included as an adaptable for the custom model.

But thanks very much for your help @mhaack , it's made it much clearer to me now.

from aem-core-cif-components.

Cool to hear you could fix the issue.

The original interface is only needed for components on the PDP or PLP, because here we have the page meta data which instantiates the Sling model already by using the original interface. That happens before your component gets called. You component then already gets the cache impl. and it is not instantiated a second time. Creating the model a second time cloud lead to the query a second time, which we want to avoid. We use Sling model caching here.

ForMyProductTeaser example this does not apply, hence here the tutorial is correct. But we should add this hint to the docs pages.

from aem-core-cif-components.

Good catch @Lawrence-M-Reynolds you are right we are not following the recommendations here. The caching of the model was introduced later this is an oversight. We will fix it.

from aem-core-cif-components.

@znikolovski Now that #507 has been merged, and based on #510 working fine with a product query extending the base product query, I think this issue is probably fixed. Can you please re-test with the current -SNAPSHOT version of the CIF components bundle?

from aem-core-cif-components.

@znikolovski any updates on this?

from aem-core-cif-components.

Hey @mhaack sorry for the late reply - this is now resolved.

from aem-core-cif-components.

@znikolovski / @cjelger , are you sure this issue is fixed? I've been trying to extend the product functionality using the delegate pattern as suggested here. But I was still seeing the same issue while using core cif components version 2.3.1, which has the change for this ticket.

After going through the pull request the only way I could see for the for it to load the custom query for the new productImpl class was to override the pagemetadata class and use my new product interface to pass to the model factory on this line:

Without this I was just seeing the same behavior as before.

from aem-core-cif-components.

@Lawrence-M-Reynolds yes this should be fixed, custom models extending the core models should work.

What is the AEM & CIF version you are using?
Can you also share the custom model class? Especially the annotations of the class?

from aem-core-cif-components.

Thanks for responding @mhaack .

The versions are:
AEM quickstart jar version: 2021.8.5788.20210830T213613Z-210800
cif: Version 2021.10.5.00

I've attached the productimpl.java aswell as the PageMetadataImpl.java I had to create and reference in the page head template.
CustomModels.zip

Maybe you can see something I've missed..

from aem-core-cif-components.

Thanks for sharing the code. What are the extension you do in the PageMetadataImpl.java I could not spot any extra methods? If you don't add extra stuff here you don't need it. Also yours looks little older, which version of CIF Core Components do you have?

For the product model you class annotations is not 100% and missing there "original" CIF core product interface.

The annotation should look like

@Model(adaptables = SlingHttpServletRequest.class, adapters = { Product.class, CustomProductModel.class}, resourceType = CustomProductModelImpl.RESOURCE_TYPE)

Where Product.class is the CIF Core Component interface and CustomProductModel.class is your extended interface. With the class will be correctly picked up by our CIF core PageMetadataImpl.java and instantiated.

from aem-core-cif-components.

Sorry to bother you again @mhaack, I was wondering if I could just run another question by you.

I came across this note on not storing the original adaptable while using sling caching as it can cause unwanted behavior:
https://sling.apache.org/documentation/bundles/models.html#a-note-about-cache--true-and-using-the-self-injector

I was considering using the same mechanism for another model that makes a graphql call and wanted to make sure I was doing it correctly. But it appears you haven't followed this recommendation for your implementation of your ProductImpl.java.

Was this just an oversight, or do you not expect this to be an issue?

from aem-core-cif-components.

Actually I was aware of the recommendation, but the main issue was fixed in sling models 1.4.10. Although the website says it may create issues, we've never had any in the past few years. Also note that this recommendation is pretty old and there have been quite a few changes in the sling caching since then, so I'm not 100% sure this still applies.

from aem-core-cif-components.