Damans227 commented on code in PR #13566:
URL: https://github.com/apache/cloudstack/pull/13566#discussion_r4205519189


##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -5329,6 +5344,122 @@ private Pair<List<TemplateJoinVO>, Integer> 
templateChecks(boolean isIso, List<H
         // about what the code was doing.
     }
 
+    /**
+     * Build the immutable filter the bypass-the-view DAO consumes. Mirrors the
+     * SearchCriteria construction in {@link #searchForTemplatesInternal}; the
+     * SearchBuilder path is unchanged. The "hard" markers (sharedAccountIds,
+     * domainIdsForFeaturedCommunity, tags, requiresViewFallback) drive
+     * {@link TemplateListFilter#canBypass()} — when any are populated,
+     * {@code templateChecks} routes to the legacy view-based path instead.
+     */
+    private TemplateListFilter buildTemplateListFilter(Long templateId, 
List<Long> ids, String name, String keyword,
+                                                       TemplateFilter 
templateFilter, boolean isIso, Boolean bootable,
+                                                       Long pageSize, Long 
startIndex, Long zoneId, HypervisorType hyperType,
+                                                       List<HypervisorType> 
hypers, boolean showDomr, boolean onlyReady,
+                                                       List<Account> 
permittedAccounts, Account caller,
+                                                       
ListProjectResourcesCriteria listProjectResourcesCriteria,
+                                                       Map<String, String> 
tags, boolean showRemovedTmpl, Long parentTemplateId,
+                                                       Boolean showUnique, 
String templateType, Boolean isVnf, Boolean forCks) {
+        TemplateListFilter.Builder b = TemplateListFilter.builder()

Review Comment:
   should the fast path also handle the arch, os category, extension, storage 
and image store filters? looks like it drops them and returns every template



##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -5329,6 +5344,122 @@ private Pair<List<TemplateJoinVO>, Integer> 
templateChecks(boolean isIso, List<H
         // about what the code was doing.
     }
 
+    /**
+     * Build the immutable filter the bypass-the-view DAO consumes. Mirrors the
+     * SearchCriteria construction in {@link #searchForTemplatesInternal}; the
+     * SearchBuilder path is unchanged. The "hard" markers (sharedAccountIds,
+     * domainIdsForFeaturedCommunity, tags, requiresViewFallback) drive
+     * {@link TemplateListFilter#canBypass()} — when any are populated,
+     * {@code templateChecks} routes to the legacy view-based path instead.
+     */
+    private TemplateListFilter buildTemplateListFilter(Long templateId, 
List<Long> ids, String name, String keyword,
+                                                       TemplateFilter 
templateFilter, boolean isIso, Boolean bootable,
+                                                       Long pageSize, Long 
startIndex, Long zoneId, HypervisorType hyperType,
+                                                       List<HypervisorType> 
hypers, boolean showDomr, boolean onlyReady,
+                                                       List<Account> 
permittedAccounts, Account caller,
+                                                       
ListProjectResourcesCriteria listProjectResourcesCriteria,
+                                                       Map<String, String> 
tags, boolean showRemovedTmpl, Long parentTemplateId,
+                                                       Boolean showUnique, 
String templateType, Boolean isVnf, Boolean forCks) {
+        TemplateListFilter.Builder b = TemplateListFilter.builder()
+                .templateId(templateId)
+                .ids(ids == null ? null : new ArrayList<>(ids))
+                .name(name)
+                .keyword(keyword)
+                .hypervisorType(hyperType != null && 
!HypervisorType.None.equals(hyperType) ? hyperType : null)
+                .availableHypervisors(hypers)
+                .format(ImageFormat.ISO)  // bypass DAO uses isIso flag to 
flip between EQ and NEQ
+                .isIso(isIso)
+                .bootable(bootable)
+                .parentTemplateId(parentTemplateId)
+                .zoneId(zoneId)
+                .onlyReady(onlyReady)
+                .excludeSystemTemplates(!showDomr)
+                .templateType(templateType)
+                .isVnf(isVnf)
+                .forCks(forCks)
+                .showUnique(showUnique != null && showUnique)
+                .startIndex(startIndex)
+                .pageSize(pageSize)
+                .sortAscending(SortKeyAscending.value())
+                .showRemoved(showRemovedTmpl)
+                .tags(tags);
+
+        if (!showRemovedTmpl) {
+            b.templateStates(Arrays.asList(VirtualMachineTemplate.State.Active,
+                    VirtualMachineTemplate.State.UploadAbandoned,
+                    VirtualMachineTemplate.State.UploadError,
+                    VirtualMachineTemplate.State.NotUploaded,
+                    VirtualMachineTemplate.State.UploadInProgress));
+        }
+
+        // accountType filter (Project vs non-Project), applied only when no 
specific templateId.
+        if (templateId == null && listProjectResourcesCriteria == 
ListProjectResourcesCriteria.SkipProjectResources) {
+            b.accountTypeNeq(Account.Type.PROJECT);
+        } else if (templateId == null && listProjectResourcesCriteria == 
ListProjectResourcesCriteria.ListProjectResourcesOnly) {
+            b.accountTypeEq(Account.Type.PROJECT);
+        }
+
+        // ACL handling — mirrors the templateFilter switch in 
searchForTemplatesInternal.
+        // For "hard" templateFilter values, set requiresViewFallback so 
canBypass() returns
+        // false and the dispatcher falls back to the legacy view-based path.
+        if (templateId != null) {
+            return b.build();
+        }
+
+        List<Long> permittedAccountIds = new ArrayList<>();
+        for (Account account : permittedAccounts) {
+            permittedAccountIds.add(account.getId());
+        }
+
+        if (templateFilter == TemplateFilter.featured || templateFilter == 
TemplateFilter.community) {
+            // Walks the domain hierarchy and ORs domainId IS NULL — not yet 
modeled in bypass SQL.
+            b.publicTemplate(Boolean.TRUE);
+            b.featured(templateFilter == TemplateFilter.featured ? 
Boolean.TRUE : Boolean.FALSE);
+            b.requiresViewFallback(true);
+        } else if (templateFilter == TemplateFilter.self || templateFilter == 
TemplateFilter.selfexecutable) {

Review Comment:
   what happens if an admin passes domainid or isrecursive=false with 
templatefilter=self? looks like the fast path ignores both



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to