Copilot commented on code in PR #6890: URL: https://github.com/apache/incubator-kie/pull/6890#discussion_r3773229273
########## drools-base/src/main/java/org/drools/base/phreak/actions/AbstractPropagationEntry.java: ########## @@ -0,0 +1,59 @@ +/* + * 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. + */ +package org.drools.base.phreak.actions; + +import org.drools.base.base.ValueResolver; +import org.drools.base.phreak.PropagationEntry; + +public abstract class AbstractPropagationEntry<T extends ValueResolver> implements PropagationEntry<T> { + protected PropagationEntry next; + + public void setNext(PropagationEntry next) { + this.next = next; + } Review Comment: AbstractPropagationEntry stores the linked-list pointer as a raw PropagationEntry and accepts a raw type in setNext(), which propagates unchecked usage through the codebase. This issue also appears in the following locations of the same file: - line 31 - line 56 ########## drools-base/src/main/java/org/drools/base/phreak/PropagationEntry.java: ########## @@ -0,0 +1,45 @@ +/** + * 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. + */ +package org.drools.base.phreak; + +import org.drools.base.base.ValueResolver; + +public interface PropagationEntry<T extends ValueResolver> { + + default void execute(T t) { + internalExecute(t); + } + + void internalExecute(T t); + + PropagationEntry getNext(); + + void setNext(PropagationEntry next); Review Comment: PropagationEntry is generic, but getNext()/setNext() use raw PropagationEntry. This forces unchecked casts in callers (e.g., propagation lists) and defeats the type-safety benefit of introducing PropagationEntry<T>. ########## drools-core/src/main/java/org/drools/core/phreak/actions/Delete.java: ########## @@ -0,0 +1,64 @@ +/* + * 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. + */ +package org.drools.core.phreak.actions; + +import org.drools.base.phreak.PropagationEntry; +import org.drools.base.phreak.actions.AbstractPropagationEntry; +import org.drools.core.common.InternalFactHandle; +import org.drools.core.common.PropagationContext; +import org.drools.core.common.ReteEvaluator; +import org.drools.core.reteoo.EntryPointNode; +import org.drools.core.reteoo.ObjectTypeConf; + +public class Delete extends AbstractPropagationEntry<ReteEvaluator> { + private final EntryPointNode epn; + private final InternalFactHandle handle; + private final PropagationContext context; + private final ObjectTypeConf objectTypeConf; + + public Delete(EntryPointNode epn, InternalFactHandle handle, PropagationContext context, ObjectTypeConf objectTypeConf) { + this.epn = epn; + this.handle = handle; + this.context = context; + this.objectTypeConf = objectTypeConf; + } + + public void internalExecute(ReteEvaluator reteEvaluator) { + execute(reteEvaluator, epn, handle, context, objectTypeConf); + } + + public static void execute(ReteEvaluator reteEvaluator, EntryPointNode epn, InternalFactHandle handle, PropagationContext context, ObjectTypeConf objectTypeConf) { + epn.propagateRetract(handle, context, objectTypeConf, reteEvaluator); + } + + @Override + public boolean isPartitionSplittable() { + return true; + } + + @Override + public PropagationEntry getSplitForPartition(int partitionNr) { + return new PartitionedDelete(handle, context, objectTypeConf, partitionNr); + } Review Comment: getSplitForPartition() should return PropagationEntry<ReteEvaluator> (not a raw PropagationEntry) to preserve the new generic type-safety and avoid unchecked usage in partitioned propagations. ########## drools-core/src/main/java/org/drools/core/phreak/actions/Insert.java: ########## @@ -0,0 +1,148 @@ +/* + * 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. + */ +package org.drools.core.phreak.actions; + +import org.drools.base.phreak.actions.AbstractPropagationEntry; +import org.drools.core.common.DefaultEventHandle; +import org.drools.core.common.InternalFactHandle; +import org.drools.core.common.PropagationContext; +import org.drools.core.common.ReteEvaluator; +import org.drools.core.impl.WorkingMemoryReteExpireAction; +import org.drools.base.phreak.PropagationEntry; +import org.drools.core.reteoo.ClassObjectTypeConf; +import org.drools.core.reteoo.ObjectTypeConf; +import org.drools.core.reteoo.ObjectTypeNode; +import org.drools.core.time.JobContext; +import org.drools.core.time.impl.DefaultJobHandle; +import org.drools.core.time.impl.PointInTimeTrigger; +import org.kie.api.prototype.PrototypeEventInstance; + +import java.io.Externalizable; +import java.io.IOException; +import java.io.ObjectInput; +import java.io.ObjectOutput; + +import static org.drools.base.rule.TypeDeclaration.NEVER_EXPIRES; + +public class Insert extends AbstractPropagationEntry<ReteEvaluator> implements Externalizable { + private static final ObjectTypeNode.ExpireJob job = new ObjectTypeNode.ExpireJob(); + + private InternalFactHandle handle; + private PropagationContext context; + private ObjectTypeConf objectTypeConf; + + public Insert() {} + + public Insert(InternalFactHandle handle, PropagationContext context, ReteEvaluator reteEvaluator, ObjectTypeConf objectTypeConf) { + this.handle = handle; + this.context = context; + this.objectTypeConf = objectTypeConf; + if (handle.isEvent()) { + scheduleExpiration(reteEvaluator, handle, context, objectTypeConf, reteEvaluator.getTimerService().getCurrentTime()); + } + } + + public static void execute(InternalFactHandle handle, PropagationContext context, ReteEvaluator reteEvaluator, ObjectTypeConf objectTypeConf) { + if (handle.isEvent()) { + scheduleExpiration(reteEvaluator, handle, context, objectTypeConf, reteEvaluator.getTimerService().getCurrentTime()); + } + propagate(handle, context, reteEvaluator, objectTypeConf); + } + + private static void propagate(InternalFactHandle handle, PropagationContext context, ReteEvaluator reteEvaluator, ObjectTypeConf objectTypeConf) { + if (objectTypeConf == null) { + objectTypeConf = handle.getEntryPoint(reteEvaluator).getObjectTypeConfigurationRegistry().getOrCreateObjectTypeConf(handle.getEntryPointId(), handle.getObject()); + } + for (ObjectTypeNode otn : objectTypeConf.getObjectTypeNodes()) { + otn.propagateAssert(handle, context, reteEvaluator); + } + if (isOrphanHandle(handle, reteEvaluator)) { + handle.setDisconnected(true); + handle.getEntryPoint(reteEvaluator).getObjectStore().removeHandle(handle); + if (handle instanceof DefaultEventHandle eventHandle) { + eventHandle.unscheduleAllJobs(reteEvaluator); + } + } + } + + private static boolean isOrphanHandle(InternalFactHandle handle, ReteEvaluator reteEvaluator) { + return !handle.hasMatches() && !reteEvaluator.getKnowledgeBase().getKieBaseConfiguration().isMutabilityEnabled(); + } + + public void internalExecute(ReteEvaluator reteEvaluator) { + propagate(handle, context, reteEvaluator, objectTypeConf); + } + + private static void scheduleExpiration(ReteEvaluator reteEvaluator, InternalFactHandle handle, PropagationContext context, ObjectTypeConf objectTypeConf, long insertionTime) { + for (ObjectTypeNode otn : objectTypeConf.getObjectTypeNodes()) { + long expirationOffset = objectTypeConf.isPrototype() ? ((PrototypeEventInstance) handle.getObject()).getExpiration() : otn.getExpirationOffset(); + scheduleExpiration(reteEvaluator, handle, context, otn, insertionTime, expirationOffset); + } + if (objectTypeConf.getConcreteObjectTypeNode() == null) { + long expirationOffset = objectTypeConf.isPrototype() ? ((PrototypeEventInstance) handle.getObject()).getExpiration() : ((ClassObjectTypeConf) objectTypeConf).getExpirationOffset(); + scheduleExpiration(reteEvaluator, handle, context, null, insertionTime, expirationOffset); + } + } + + private static void scheduleExpiration(ReteEvaluator reteEvaluator, InternalFactHandle handle, PropagationContext context, ObjectTypeNode otn, long insertionTime, long expirationOffset) { + if (expirationOffset == NEVER_EXPIRES || expirationOffset == Long.MAX_VALUE || context.getReaderContext() != null) { + return; + } + DefaultEventHandle eventFactHandle = (DefaultEventHandle) handle; + long nextTimestamp = getNextTimestamp(insertionTime, expirationOffset, eventFactHandle); + WorkingMemoryReteExpireAction action = new WorkingMemoryReteExpireAction((DefaultEventHandle) handle, otn); + if (nextTimestamp <= reteEvaluator.getTimerService().getCurrentTime()) { + reteEvaluator.addPropagation(action); + } else { + JobContext jobctx = new ObjectTypeNode.ExpireJobContext(action, reteEvaluator); + DefaultJobHandle jobHandle = (DefaultJobHandle) reteEvaluator.getTimerService() + .scheduleJob(job, jobctx, PointInTimeTrigger.createPointInTimeTrigger(nextTimestamp, null)); + jobctx.setJobHandle(jobHandle); + eventFactHandle.addJob(jobHandle); + } + } + + private static long getNextTimestamp(long insertionTime, long expirationOffset, DefaultEventHandle eventFactHandle) { + long effectiveEnd = eventFactHandle.getEndTimestamp() + expirationOffset; + return Math.max(insertionTime, effectiveEnd >= 0 ? effectiveEnd : Long.MAX_VALUE); + } + + @Override + public String toString() { + return "Insert of " + handle.getObject(); + } + + public InternalFactHandle getHandle() { + return handle; + } + + @Override + public void writeExternal(ObjectOutput out) throws IOException { + out.writeObject(next); + out.writeObject(handle); + out.writeObject(context); + } + + @Override + public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { + this.next = (PropagationEntry) in.readObject(); + this.handle = (InternalFactHandle) in.readObject(); + this.context = (PropagationContext) in.readObject(); Review Comment: After making AbstractPropagationEntry.next generic, this deserialization assignment should cast to PropagationEntry<ReteEvaluator> and suppress the unavoidable unchecked warning locally (instead of using a raw PropagationEntry cast). ########## drools-core/src/main/java/org/drools/core/phreak/actions/Update.java: ########## @@ -0,0 +1,101 @@ +/* + * 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. + */ +package org.drools.core.phreak.actions; + +import org.drools.base.phreak.PropagationEntry; +import org.drools.base.phreak.actions.AbstractPropagationEntry; +import org.drools.core.common.InternalFactHandle; +import org.drools.core.common.PropagationContext; +import org.drools.core.common.ReteEvaluator; +import org.drools.core.reteoo.ModifyPreviousTuples; +import org.drools.core.reteoo.ObjectTypeConf; +import org.drools.core.reteoo.ObjectTypeNode; + +import java.io.Externalizable; +import java.io.IOException; +import java.io.ObjectInput; +import java.io.ObjectOutput; + +import static org.drools.core.reteoo.EntryPointNode.removeRightTuplesMatchingOTN; + +public class Update extends AbstractPropagationEntry<ReteEvaluator> implements Externalizable { + private InternalFactHandle handle; + private PropagationContext context; + private ObjectTypeConf objectTypeConf; + + public Update() {} + + public Update(InternalFactHandle handle, PropagationContext context, ObjectTypeConf objectTypeConf) { + this.handle = handle; + this.context = context; + this.objectTypeConf = objectTypeConf; + } + + public void internalExecute(ReteEvaluator reteEvaluator) { + execute(handle, context, objectTypeConf, reteEvaluator); + } + + public InternalFactHandle getHandle() { + return handle; + } + + public static void execute(InternalFactHandle handle, PropagationContext pctx, ObjectTypeConf objectTypeConf, ReteEvaluator reteEvaluator) { + if (objectTypeConf == null) { + objectTypeConf = handle.getEntryPoint(reteEvaluator).getObjectTypeConfigurationRegistry().getOrCreateObjectTypeConf(handle.getEntryPointId(), handle.getObject()); + } + ModifyPreviousTuples modifyPreviousTuples = new ModifyPreviousTuples(handle.detachLinkedTuples()); + ObjectTypeNode[] cachedNodes = objectTypeConf.getObjectTypeNodes(); + for (int i = 0, length = cachedNodes.length; i < length; i++) { + cachedNodes[i].modifyObject(handle, modifyPreviousTuples, pctx, reteEvaluator); + if (i < cachedNodes.length - 1) { + removeRightTuplesMatchingOTN(pctx, reteEvaluator, modifyPreviousTuples, cachedNodes[i], 0); + } + } + modifyPreviousTuples.retractTuples(pctx, reteEvaluator); + } + + @Override + public boolean isPartitionSplittable() { + return true; + } + + @Override + public PropagationEntry getSplitForPartition(int partitionNr) { + return new PartitionedUpdate(handle, context, objectTypeConf, partitionNr); + } + + @Override + public String toString() { + return "Update of " + handle.getObject(); + } + + @Override + public void writeExternal(ObjectOutput out) throws IOException { + out.writeObject(next); + out.writeObject(handle); + out.writeObject(context); + } + + @Override + public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { + this.next = (PropagationEntry) in.readObject(); + this.handle = (InternalFactHandle) in.readObject(); + this.context = (PropagationContext) in.readObject(); Review Comment: After making AbstractPropagationEntry.next generic, this deserialization assignment should cast to PropagationEntry<ReteEvaluator> and suppress the unavoidable unchecked warning locally (instead of using a raw PropagationEntry cast). ########## drools-core/src/main/java/org/drools/core/phreak/actions/Update.java: ########## @@ -0,0 +1,101 @@ +/* + * 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. + */ +package org.drools.core.phreak.actions; + +import org.drools.base.phreak.PropagationEntry; +import org.drools.base.phreak.actions.AbstractPropagationEntry; +import org.drools.core.common.InternalFactHandle; +import org.drools.core.common.PropagationContext; +import org.drools.core.common.ReteEvaluator; +import org.drools.core.reteoo.ModifyPreviousTuples; +import org.drools.core.reteoo.ObjectTypeConf; +import org.drools.core.reteoo.ObjectTypeNode; + +import java.io.Externalizable; +import java.io.IOException; +import java.io.ObjectInput; +import java.io.ObjectOutput; + +import static org.drools.core.reteoo.EntryPointNode.removeRightTuplesMatchingOTN; + +public class Update extends AbstractPropagationEntry<ReteEvaluator> implements Externalizable { + private InternalFactHandle handle; + private PropagationContext context; + private ObjectTypeConf objectTypeConf; + + public Update() {} + + public Update(InternalFactHandle handle, PropagationContext context, ObjectTypeConf objectTypeConf) { + this.handle = handle; + this.context = context; + this.objectTypeConf = objectTypeConf; + } + + public void internalExecute(ReteEvaluator reteEvaluator) { + execute(handle, context, objectTypeConf, reteEvaluator); + } + + public InternalFactHandle getHandle() { + return handle; + } + + public static void execute(InternalFactHandle handle, PropagationContext pctx, ObjectTypeConf objectTypeConf, ReteEvaluator reteEvaluator) { + if (objectTypeConf == null) { + objectTypeConf = handle.getEntryPoint(reteEvaluator).getObjectTypeConfigurationRegistry().getOrCreateObjectTypeConf(handle.getEntryPointId(), handle.getObject()); + } + ModifyPreviousTuples modifyPreviousTuples = new ModifyPreviousTuples(handle.detachLinkedTuples()); + ObjectTypeNode[] cachedNodes = objectTypeConf.getObjectTypeNodes(); + for (int i = 0, length = cachedNodes.length; i < length; i++) { + cachedNodes[i].modifyObject(handle, modifyPreviousTuples, pctx, reteEvaluator); + if (i < cachedNodes.length - 1) { + removeRightTuplesMatchingOTN(pctx, reteEvaluator, modifyPreviousTuples, cachedNodes[i], 0); + } + } + modifyPreviousTuples.retractTuples(pctx, reteEvaluator); + } + + @Override + public boolean isPartitionSplittable() { + return true; + } + + @Override + public PropagationEntry getSplitForPartition(int partitionNr) { + return new PartitionedUpdate(handle, context, objectTypeConf, partitionNr); + } Review Comment: getSplitForPartition() should return PropagationEntry<ReteEvaluator> (not a raw PropagationEntry) to preserve the new generic type-safety and avoid unchecked usage in partitioned propagations. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
