Xiao-zhen-Liu commented on code in PR #8640: URL: https://github.com/apache/texera/pull/8640#discussion_r4261812818
########## frontend/src/app/workspace/service/compile-workflow/loop-block.service.ts: ########## @@ -0,0 +1,94 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Injectable } from "@angular/core"; +import { Observable, Subject } from "rxjs"; + +/** + * Which Loop Starts enclose each operator, as the latest editing-time compile reports it. The backend + * works it out from the logical plan it compiles, and WorkflowCompilingService records here the + * `operatorLoopStarts` of every compile response, successful or not. + * + * Two readers look again after every compile result: the property panel, which offers `$name` + * loop-variable references only inside a loop block, and ValidationWorkflowService, which waives the + * type check at such a reference only there. Every edit that can move an operator into or out of a block, + * or change the variables a Loop Start declares, is followed by a compile, so neither needs to know which + * edits those are. + * + * The compile is sent only the operators ValidationWorkflowService judges valid, so while one is left out, + * as an operator in a block is while it holds "$" on the way to "$K", a compile result misses the blocks + * around that operator, and around the ones whose path to their Loop End runs through it. Such a result + * does not take an operator out of a block; see setOperatorLoopStarts. + * + * Until the first compile result, no operator counts as inside a loop block. + * + * A service of its own rather than part of WorkflowCompilingService because ValidationWorkflowService, + * which WorkflowCompilingService injects, reads it. + */ +@Injectable({ + providedIn: "root", +}) +export class LoopBlockService { + private operatorLoopStarts: ReadonlyMap<string, ReadonlyArray<string>> = new Map(); + private readonly compileResultStream = new Subject<void>(); + + /** + * Records the `operatorLoopStarts` of a compile response and announces it. + * + * A compile of the whole workflow replaces the previous result. What a compile that was not sent every + * operator reports still holds, since each path it sees is one in the workflow and one path is enough to + * put an operator inside a block; but an operator it finds outside every block may not be. So each + * operator also keeps the blocks the previous result put it in, until a compile of the whole workflow. + * @param operatorLoopStarts for each operator inside a loop block, the Loop Starts of the blocks it is + * inside; an operator outside every block is absent + * @param wholeWorkflow whether the compile was sent every operator of the workflow + */ + public setOperatorLoopStarts( + operatorLoopStarts: Readonly<Record<string, ReadonlyArray<string>>>, + wholeWorkflow = true + ): void { + const reported = new Map(Object.entries(operatorLoopStarts)); + if (!wholeWorkflow) { Review Comment: While any operator in the workflow is invalid, an operator can't leave a loop here, because the compile result is merged with the previous one. For example, with an unfinished Filter elsewhere on the canvas, moving a Limit that holds `$i` out of its loop leaves its panel offering `$i` and showing no type error; only the backend's compile error says otherwise. Which loop an operator is in depends only on operator types and links, so I think it should be worked out from the whole graph, valid or not, instead of remembering old results. -- 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]
