Eli Mesika has posted comments on this change. Change subject: core : Support tokens with special characters ......................................................................
Patch Set 1: (6 comments) https://gerrit.ovirt.org/#/c/41832/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SearchQuery.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SearchQuery.java: Line 63: Line 64: public SearchQuery(P parameters) { Line 65: super(parameters); Line 66: } Line 67: > What about using regex to indetify AD* query: Done Line 68: @Override Line 69: protected void executeQueryCommand() { Line 70: List<? extends IVdcQueryable> returnValue = new ArrayList<>(); Line 71: switch (getParameters().getSearchTypeValue()) { Line 357: } Line 358: // query not in cache or the cached entry is too old, process the Line 359: // search text. Line 360: if (!isExistsValue || IsFromYesterday) { Line 361: log.debug("ResourceManager::searchBusinessObjects(''{}'') - entered", searchText); > I'd put different parts below in their own methods, to clarify better the l Keep in mind that those blocks set more than 1 var value so I doubt it will be more clear than that ... I will add explanation Line 362: final char AT='@'; Line 363: String queryAuthz = getDefaultAuthz(); Line 364: String queryNamespace = null; Line 365: ISyntaxChecker curSyntaxChecker; Line 372: searchText = searchText.replace(SearchObjects.AD_USER_PLU_OBJ_NAME + AT, StringUtils.EMPTY); Line 373: searchText = searchText.replace(SearchObjects.AD_USER_OBJ_NAME + AT, StringUtils.EMPTY); Line 374: searchText = searchText.replace(SearchObjects.AD_GROUP_PLU_OBJ_NAME + AT, StringUtils.EMPTY); Line 375: searchText = searchText.replace(SearchObjects.AD_GROUP_OBJ_NAME + AT, StringUtils.EMPTY); Line 376: > Instead of lines 366-375 just use these: Done Line 377: // get profile Line 378: List<String> profiles = getBackend().runInternalQuery(VdcQueryType.GetDomainList, Line 379: new VdcQueryParametersBase()).getReturnValue(); Line 380: for (String profile : profiles) { Line 382: queryAuthz = profile; Line 383: searchText = searchText.replace(profile + COLON, StringUtils.EMPTY); Line 384: break; Line 385: } Line 386: } > I think we could use pattern matching also for profiles and namespaces, but not in the scope of this patch IMO I would like to do a minimal change that can be easily back ported to 3.5 Line 387: // get namespace Line 388: HashMap<String, List<String>> namespacesMap = Line 389: getBackend().runInternalQuery(VdcQueryType.GetAvailableNamespaces, Line 390: new VdcQueryParametersBase()).getReturnValue(); Line 398: } Line 399: searchText = prefix + searchText; Line 400: curSyntaxChecker = SyntaxCheckerFactory.createADSyntaxChecker(Config Line 401: .<String>getValue(ConfigValues.AuthenticationMethod)); Line 402: > Why adding empty line? Done Line 403: } else { Line 404: curSyntaxChecker = SyntaxCheckerFactory Line 405: .createBackendSyntaxChecker(Config.<String>getValue(ConfigValues.AuthenticationMethod)); Line 406: } Line 403: } else { Line 404: curSyntaxChecker = SyntaxCheckerFactory Line 405: .createBackendSyntaxChecker(Config.<String>getValue(ConfigValues.AuthenticationMethod)); Line 406: } Line 407: > Why adding empty line? Done Line 408: SyntaxContainer searchObj = curSyntaxChecker.analyzeSyntaxState(searchText, true); Line 409: // set the case-sensitive flag Line 410: searchObj.setCaseSensitive(getParameters().getCaseSensitive()); Line 411: // If a number > maxValue is given then maxValue will be used -- To view, visit https://gerrit.ovirt.org/41832 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2464227376eb2e6ee0b5ada2ced21278675b7572 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
