Modifications in order to make multiple classes become public #310

alexmustata19 · 2024-03-15T15:18:07Z

No description provided.

gabriela-lungu-uip

The framework for these types of changes is as follows:

transition the class from internal to public
transition back all items from the class that were marked as public to internal (and do not need to be public for the current GEH task)

This way we will many more code changes but less functional/interface ones

src/UiPath.Workflow.Runtime/ActivityUtilities.cs

src/UiPath.Workflow.Runtime/ProcessActivityTreeOptions.cs

src/UiPath.Workflow.Runtime/Runtime/CallbackWrapper.cs

src/UiPath.Workflow.Runtime/Runtime/CompletionBookmark.cs

src/UiPath.Workflow.Runtime/Runtime/CompletionCallbackWrapper.cs

src/UiPath.Workflow.Runtime/Runtime/FaultBookmark.cs

src/UiPath.Workflow.Runtime/Runtime/FaultCallbackWrapper.cs

gabriela-lungu-uip · 2024-04-25T14:58:01Z

src/UiPath.Workflow.Runtime/ActivityUtilities.cs

@@ -12,7 +12,7 @@ namespace System.Activities;
 using Runtime;
 using Validation;

-internal static class ActivityUtilities


there are still some methods public here, do you need them all?

CacheRootMetadata must remain public in order to not break reflection implementation in other projects.
We use CreateFaultBookmark(FaultCallback onFaulted, ActivityInstance owningInstance) & CreateCompletionBookmark(CompletionCallback onCompleted, ActivityInstance owningInstance) directly so they need to remain public.
Overloads of CreateCompletionBookmark were left also public.

gabriela-lungu-uip · 2024-04-25T14:58:50Z

src/UiPath.Workflow.Runtime/ActivityUtilities.cs

@@ -1158,18 +1158,18 @@ internal override void OnInternalCacheMetadata(bool createEmptyBindings)

    public struct ChildActivity : IEquatable<ChildActivity>


make only ChildActivity internal, and all items in it can remain unchanged

CacheRootMetadata method needs to be public. For this to happen ProcessActivityCallback needs to be public which implies that ChildActivity and ActivityCallStack needs to be both public.

gabriela-lungu-uip · 2024-04-25T15:00:06Z

src/UiPath.Workflow.Runtime/ActivityUtilities.cs

@@ -1184,18 +1184,18 @@ public class ActivityCallStack
        private int _nonExecutingParentCount;
        private readonly Quack<ChildActivity> _callStack;

-        public ActivityCallStack()


change ActivityCallStack to internal and all items in it can remain unchanged

Explained in ChildActivity comment the situation.

vuplea · 2024-05-10T15:15:05Z

src/UiPath.Workflow.Runtime/ActivityUtilitiesWrapper.cs

@@ -0,0 +1,12 @@
+using System.Activities.Runtime;


we don't need to expose these methods for the current PR on Studio, but they might be useful to replace the reflection used for exception handler at some point.

Removed this class. Fixed.

vuplea · 2024-05-10T15:18:38Z

src/UiPath.Workflow.Runtime/Runtime/CompletionCallbackWrapper.cs

 {
    private bool _checkForCancelation;
    private bool _needsToGatherOutputs;

-    protected CompletionCallbackWrapper(Delegate callback, ActivityInstance owningInstance)
+    public CompletionCallbackWrapper(Delegate callback, ActivityInstance owningInstance)


This change doesn't do anything since it's an abstract class.
protected rather than public is better from a readability perspective.

Reverted the constructor back to protected. Fixed.

…lizable json properties.

…o attribute.

… internals.

…ore public.

…stanceStore public." This reverts commit 68a521f.

… to be compatible with GEH Json Persistence requirements.

…ssActivityTreeOptions class to be public.

… CallbackWrapper, CompletionCallbackWrapper, FaultCallbackWrapper, CompletionBookmark and FaultBookmark classes.

…tionCallbackWrapper, FaultCallbackWrapper, CompletionBookmark and FaultBookmark classes.

…, CompletionCallbackWrapper, FaultCallbackWrapper, CompletionBookmark and FaultBookmark classes." This reverts commit 232b841.

… the ActivityUtilities class FinishCachingSubtree & ProcessActivityInstanceTree methods.

…o public the ActivityUtilities class FinishCachingSubtree & ProcessActivityInstanceTree methods." This reverts commit 8f7665a.

…CompletionBookmark methods of ActivityUtilities static class.

…blic property.

gabriela-lungu-uip · 2024-05-15T13:02:01Z

src/UiPath.Workflow.Runtime/Runtime/CallbackWrapper.cs

-    public ActivityInstance ActivityInstance
+    public Delegate Callback { get; set; }
+
+    internal ActivityInstance ActivityInstance


this is accessed with reflexion. are we sure we did not break it?

Reverted ActivityInstance property to be public.

gabriela-lungu-uip · 2024-05-15T14:23:31Z

src/UiPath.Workflow.Runtime/Runtime/CompletionBookmark.cs

@@ -4,7 +4,7 @@
 namespace System.Activities.Runtime;

 [DataContract]
-internal class CompletionBookmark
+public class CompletionBookmark


do you still need this to be public?

Fixed. Reverted to internal.

gabriela-lungu-uip · 2024-05-15T14:23:59Z

src/UiPath.Workflow.Runtime/Runtime/CompletionCallbackWrapper.cs

@@ -7,7 +7,7 @@ namespace System.Activities.Runtime;
 [KnownType(typeof(ActivityCompletionCallbackWrapper))]
 [KnownType(typeof(DelegateCompletionCallbackWrapper))]
 [DataContract]
-internal abstract class CompletionCallbackWrapper : CallbackWrapper
+public abstract class CompletionCallbackWrapper : CallbackWrapper


do you still need this to be public?

Fixed. Reverted to internal.

gabriela-lungu-uip · 2024-05-15T14:24:34Z

src/UiPath.Workflow.Runtime/Runtime/FaultBookmark.cs

@@ -4,7 +4,7 @@
 namespace System.Activities.Runtime;

 [DataContract]
-internal class FaultBookmark
+public class FaultBookmark


do you still need this to be public?

Fixed. Reverted to internal.

…per classes to be internal.

vuplea · 2024-05-16T07:53:16Z

src/UiPath.Workflow.Runtime/Runtime/CallbackWrapper.cs

    private ActivityInstance _activityInstance;

    protected internal CallbackWrapper() { }

    public CallbackWrapper(Delegate callback, ActivityInstance owningInstance)
    {
        ActivityInstance = owningInstance;
-        _callback = callback;
+        Callback = callback;


The rename of the backing field is also a risk.

Added back backing field _callback.

vuplea · 2024-05-16T07:54:14Z

src/UiPath.Workflow.Runtime/Runtime/CallbackWrapper.cs

@@ -56,13 +55,13 @@ internal string SerializedDeclaringTypeName
    }

    [DataMember(Name = "ActivityInstance")]
-    internal ActivityInstance SerializedActivityInstance
+    public ActivityInstance SerializedActivityInstance


Is this needed as public?

Please go through the PRs again, making sure every change can be justified.

Reverted SerializedActivityInstance property to be internal, because isn't used anywhere in our repo.

…e get and set methods that operate on the _callback backing field. Reverted the usages in the class tto _callback. Reverted SerializedActivityInstance property to be internal. Reverted ActivityInstance property to be public.

gabriela-lungu-uip · 2024-05-20T14:08:22Z

src/UiPath.Workflow.Runtime/Runtime/CompletionBookmark.cs

@@ -19,19 +19,19 @@ public CompletionBookmark(CompletionCallbackWrapper callbackWrapper)
    }

    [DataMember(EmitDefaultValue = false, Name = "callbackWrapper")]
-    internal CompletionCallbackWrapper SerializedCallbackWrapper
+    public CompletionCallbackWrapper SerializedCallbackWrapper


changes to this file are no longer needed

gabriela-lungu-uip · 2024-05-20T14:12:04Z

src/UiPath.Workflow.Runtime/Runtime/CompletionCallbackWrapper.cs

@@ -35,7 +35,7 @@ internal bool SerializedNeedsToGatherOutputs
        set => _needsToGatherOutputs = value;
    }

-    public void CheckForCancelation() => _checkForCancelation = true;
+    internal void CheckForCancelation() => _checkForCancelation = true;


changes to this file are no longer needed

gabriela-lungu-uip · 2024-05-20T14:13:13Z

src/UiPath.Workflow.Runtime/Runtime/FaultBookmark.cs

@@ -14,12 +14,12 @@ public FaultBookmark(FaultCallbackWrapper callbackWrapper)
    }

    [DataMember(Name = "callbackWrapper")]
-    internal FaultCallbackWrapper SerializedCallbackWrapper


changes to this file are no longer needed

…tBookmark.cs files.

alexmustata19 changed the title ~~[WIP] Multiple public constructors~~ [WIP] Multiple public classes Mar 27, 2024

alexmustata19 changed the title ~~[WIP] Multiple public classes~~ Modifications in order to make multiple classes become public Apr 2, 2024

alexmustata19 force-pushed the feature/multiple_public_constructors branch from a041819 to 5d160a1 Compare April 2, 2024 09:37

alexmustata19 marked this pull request as ready for review April 11, 2024 08:26

alexmustata19 requested review from aoltean16 and gabriela-lungu-uip as code owners April 11, 2024 08:26

alexmustata19 force-pushed the feature/multiple_public_constructors branch from af9c744 to f6d193a Compare April 15, 2024 06:50

gabriela-lungu-uip reviewed Apr 25, 2024

View reviewed changes

alexmustata19 force-pushed the feature/multiple_public_constructors branch from 53a08bc to 54107c0 Compare April 26, 2024 08:11

gabriela-lungu-uip approved these changes Apr 26, 2024

View reviewed changes

vuplea reviewed May 10, 2024

View reviewed changes

alexmustata19 added 18 commits May 13, 2024 11:29

Work in progress.

425f64b

Made several classes public.

4f56e1a

Made ActivityCompletionCallbackWrapper class public.

f957248

Modified ActivityInstance class to have public constructors and seria…

f17abd6

…lizable json properties.

Deleted JsonProperty attributes on fields of ActivityInstance class.

350626e

Removed Newtonsoft.Json dependency from UiPath.Workflow.Runtime.

de60670

Attempt to use internal constructors for ActivityInstance.

ee272f4

Modified several properties to be public.

857df26

Modified two more classes to be public.

3fab38d

Reverted classes to be internal and attempted to use InternalVisibleT…

1676dde

…o attribute.

Added UiPath.Executor.Persistence to the list of that can observe the…

b2ced80

… internals.

Removed UiPath.Executor.Persistence InternalVisibleTo attribute.

cfd3816

Attempt to remove InternalsVisibleTo atribute from UiPath.Workflow.

b9ae581

Attempt to make BeginTryCommand & EndTryCommand methods of InstanceSt…

78cec5e

…ore public.

Revert "Attempt to make BeginTryCommand & EndTryCommand methods of In…

74d05f6

…stanceStore public." This reverts commit 68a521f.

Deleted InternalsVisibleTo attribute.

8a36d96

Removed unused using.

0615f19

Switched to using InternalsVisibleTo.

b8c3365

alexmustata19 added 15 commits May 13, 2024 11:29

Removed InternalVisibleTo attribute. Modified CoreWF in a minimal way…

445ea4e

… to be compatible with GEH Json Persistence requirements.

Made ActivityUtilities class public.

1021d3c

Modified ActivityUtilities class's CacheRootMetadata method and Proce…

f41e44b

…ssActivityTreeOptions class to be public.

Decreased visibility of certain fields, methods and constructors from…

f728083

… CallbackWrapper, CompletionCallbackWrapper, FaultCallbackWrapper, CompletionBookmark and FaultBookmark classes.

Reverted access modifiers for constructors of CallbackWrapper, Comple…

afc2aee

…tionCallbackWrapper, FaultCallbackWrapper, CompletionBookmark and FaultBookmark classes.

Revert "Reverted access modifiers for constructors of CallbackWrapper…

a2f3bdd

…, CompletionCallbackWrapper, FaultCallbackWrapper, CompletionBookmark and FaultBookmark classes." This reverts commit 232b841.

Changed access modifiers present in ActivityUtilities static class.

249c9eb

Switched constructors back to being public.

20d15b7

Reverted access modifiers from ActivityUtilities static class.

afccf39

Applied all necessary modifications in order to revert back to public…

49b76fc

… the ActivityUtilities class FinishCachingSubtree & ProcessActivityInstanceTree methods.

Revert "Applied all necessary modifications in order to revert back t…

e84bde5

…o public the ActivityUtilities class FinishCachingSubtree & ProcessActivityInstanceTree methods." This reverts commit 8f7665a.

Reverted ActivityUtilities class.

2c80900

Create a public static wrapper class for CreateFaultBookmark & Create…

def06bb

…CompletionBookmark methods of ActivityUtilities static class.

Made modified _callback field of CallbackWrapper class to become a pu…

18850bf

…blic property.

Reverted CompletionCallbackWrapper's constructor back to protected.

31a6123

alexmustata19 force-pushed the feature/multiple_public_constructors branch from b668ca5 to 31a6123 Compare May 13, 2024 08:29

Deleted ActivityUtilitiesWrapper class.

b8522d0

gabriela-lungu-uip reviewed May 15, 2024

View reviewed changes

Modified FaultBookmark, CompletionBookmark and CompletionCallbackWrap…

e2b68f3

…per classes to be internal.

vuplea reviewed May 16, 2024

View reviewed changes

gabriela-lungu-uip reviewed May 20, 2024

View reviewed changes

Reverted CompletionBookmark.cs, CompletionCallbackWrapper.cs and Faul…

be0b222

…tBookmark.cs files.

gabriela-lungu-uip approved these changes May 21, 2024

View reviewed changes

gabriela-lungu-uip merged commit bfa4a60 into develop May 21, 2024
2 checks passed

gabriela-lungu-uip deleted the feature/multiple_public_constructors branch May 21, 2024 14:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modifications in order to make multiple classes become public #310

Modifications in order to make multiple classes become public #310

alexmustata19 commented Mar 15, 2024

gabriela-lungu-uip left a comment

gabriela-lungu-uip Apr 25, 2024

alexmustata19 Apr 26, 2024

gabriela-lungu-uip Apr 25, 2024

alexmustata19 Apr 26, 2024

gabriela-lungu-uip Apr 25, 2024

alexmustata19 Apr 26, 2024

vuplea May 10, 2024

alexmustata19 May 15, 2024

vuplea May 10, 2024

alexmustata19 May 13, 2024

gabriela-lungu-uip May 15, 2024

alexmustata19 May 16, 2024

gabriela-lungu-uip May 15, 2024

alexmustata19 May 16, 2024

gabriela-lungu-uip May 15, 2024

alexmustata19 May 16, 2024 •

edited

gabriela-lungu-uip May 15, 2024

alexmustata19 May 16, 2024 •

edited

vuplea May 16, 2024

alexmustata19 May 16, 2024

vuplea May 16, 2024

alexmustata19 May 16, 2024 •

edited

gabriela-lungu-uip May 20, 2024

alexmustata19 May 21, 2024

gabriela-lungu-uip May 20, 2024

alexmustata19 May 21, 2024

gabriela-lungu-uip May 20, 2024

alexmustata19 May 21, 2024

		@@ -1158,18 +1158,18 @@ internal override void OnInternalCacheMetadata(bool createEmptyBindings)

		public struct ChildActivity : IEquatable<ChildActivity>

Modifications in order to make multiple classes become public #310

Modifications in order to make multiple classes become public #310

Conversation

alexmustata19 commented Mar 15, 2024

gabriela-lungu-uip left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexmustata19 May 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexmustata19 May 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexmustata19 May 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexmustata19 May 16, 2024 •

edited

alexmustata19 May 16, 2024 •

edited

alexmustata19 May 16, 2024 •

edited