Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate receiver resource selection logic to use new resource implementation (ref-point migration 6/10) #1037

Merged

Conversation

tobous
Copy link
Member

@tobous tobous commented Jun 22, 2020

Migrates the logic of Saros/E to select which resources to use to represent the shared reference points on the receiving side to use the new resource model introduced in #1023. Outdated javadoc and method names that are not directly related to the changed logic will be updated in a separate clean-up PR at the end.

Rewrites the wizard to select the local representation of shared reference points for incoming resource negotiations. This is done by updating the three classes representing the wizard sequentially.

With the new wizard now using reference points, the tab names are not necessarily unique anymore. Furthermore, with reference points now representing any directory in the workspace, it is much harder for the user to determine which resource the reference point is supposed to represent. See #1042.

The new wizard has scalability issues on my development machine. These issues were, however, are not reproducible on my Windows machine, meaning this is probably due to my local setup (Linux, I3-WM, 144 DPI). See #1043.

Reviewing this PR

I would strongly suggest reviewing this PR commit by commit to make this PR managable.

Commits

[UI][E] Rework ProjectOptionComposite

Reworks the project option composite to work with the new resource
model. To keep the changes as small as possible, the new handling is not
yet used. Instead, the old methods are marked as deprecated. Their usage
will be replaced in a separate commit reworking the EnterProjectNamePage
(the class containing all ProjectOptionComposites for all shared
reference points).

Adjusts the composite to have three option for the user to choose:

  • create a new project
  • create a new directory
  • use an existing directory

Adds a unified handling to select a specific option programmatically and
disable all other fields.

Adds separate methods to set a specific option/input of the dialog.

Adjusts the "browse" dialog to actually return the path of the chosen
resource instead of always returning the base project for the resource.
Replaces the text of the button with the Eclipse folder icon.

Adjusts the text shown in the composite to match the new content.

Adds ReferencePointOptionResult as a data holder class for the result of
the page. This makes it easier to pass the selected option to other
classes.

[REFACTOR][E] Rename ProjectOptionComposite and ProjectOptionListener

Renames ProjectOptionComposite to ReferencePointOptionComposite.

Renames ProjectOptionListener to ReferencePointOptionListener. Renames
the listener methods to match the new name. Adds javadoc.

[REFACTOR][E] Rename messages belonging to ReferencePointOptionComposite

[UI][E] Add tooltips to ReferencePointOptionComposite

[NOP][E] Ignore "UnusedProperty" warning in messages.properties

Adds a comment to ignore the "Unused Property" warning in the
messages.properties files. This is necessary as IntelliJ does not detect
the fields as being used as they are injected through the use of
reflections.

[INTERNAL][E] Add methods to obtain container handle for selection

Adds methods to obtain the container handle for the selection
represented by the reference point option result.

Adds IllegalInputException. The exception is used to signal that the
input represented by the result is not valid. The reason why the result
is invalid is passed as the exception message.

The added error messages will be displayed as an error in
EnterProjectNamePage, so it is important that they are concise and also
mention which reference point they belong to.

[INTERNAL][E] Rework EnterProjectNamePage

Rewrites EnterProjectNamePage to better match the new resource sharing
model and the new ReferencePointOptionComposite design.

Updates the validation logic to match the new input fields. Trims the
input strings to avoid issues where values were seen as different due to
leading or trailing spaces that are dropped during the resource
conversion, leading to resource clashes.

Updates the shown error messages to make it clearer which reference
point composite they belong to.

Updates the error message logic to always display the error message of
the currently selected tab (if present). This makes it much easier for
the user to determine what the issue with the current input actually is.
Furthermore, if the error messages are updated and there is no error for
the current tab, the error message is only changed if the currently
displayed message is no longer valid. This was done to avoid potentially
confusing the user be unnecessarily switching the error message even
though both messages are not related to the current tab.

Updates the preselection logic. The new logic now always selects the
best guess, even if it is not a valid selection. This approach was
chosen as it forces the user to manually deal with potential conflicts
instead of just giving an easy alternative. This should hopefully give
the user a better understanding of what the outcome of the resource
negotiation will be.
Updates the preselection logic to also suggest using an existing project
if a shared reference point has the same name. This heuristic was chosen
as project names should be somewhat unique in a normal project.
Furthermore, sharing entire projects should be a common use case, so
making it as easy to do so should increase the usability of the
preselection logic.

The preselection logic will also be extended in a future PR by using the
workspace relative path on the sender side to provide the basis for a
good guess on how to map the reference point locally.

Updates the text displayed in the composite, the variable and method
names, and the javadoc to match the new resource sharing model.

[INTERNAL][E] Add checks to avoid nested reference points

Adds checks to EnterProjectNamePage to ensure that the chosen local
representations will not lead to nested reference points.

[REFACTOR][E] Rename EnterProjectNamePage

Renames EnterProjectNamePage to LocalRepresentationSelectionPage to
better represent the purpose of the page.

Subsequently renames the entries in Messages and messages.properties.

[INTERNAL] Migrate AddProjectToSessionWizard

Migrates AddProjectToSessionWizard to work with the new resource model
and more specifically with the new implementation of
LocalRepresentationSelectionPage and ReferencePointOptionComposite.

Upgrades the text shown to the user to represent the new resource
sharing model.

[REFACTOR][E] Clean up AddProjectToSessionWizard

Removes unnecessary "final" declarations in the method context.

Removes unnecessary explicit argument types.

Replaces anonymous inner classes with lambda expressions.

Uses removeIf(...) to remove objects from a map. (Done automatically by
IntelliJ.)

Renames remaining local variables to match the new resource model.

[REFACTOR][E] Rename AddProjectToSessionWizard

Renames AddProjectToSessionWizard to AddReferencePointsToSessionWizard.

Subsequently renames the entries in Messages and messages.properties.

Adjusts reference to previous name in STF Constants. This is only a
hotfix to keep the STF test launch profiles from crashing. The proper
adjustment of the STF is done separately.

[NOP][E] Remove remaining deprecated old UI methods

Removes the remaining deprecated methods remaining from the old UI
implementation in ReferencePointOptionComposite and
LocalRepresentationSelectionPage as they are no longer used.

[INTERNAL][E] Remove logic to preselect already shared reference points

Removes the logic to preselect the mapping for already shared reference
points from the local representation selection page.

The logic was necessary in cases where the mapping of a partial sharing
was expanded. With partial sharing no longer being supported, the logic
is no longer needed as the incoming resource negotiation should always
only contain non-shared reference points.

@tobous tobous added Aspect: GUI Issues specific to the Saros GUI Area: Eclipse Issue affecting Saros for Eclipse (Saros/E) Breaks Compatibility Change breaking compatibility with previous releases labels Jun 22, 2020
@tobous tobous added this to the Saros/E Release NEXT milestone Jun 22, 2020
@tobous tobous self-assigned this Jun 22, 2020
@tobous tobous force-pushed the pr/eclipse/ref-point-migration/5/ui-internals-2 branch 2 times, most recently from d2cb170 to b7d0562 Compare June 25, 2020 00:15
@tobous tobous force-pushed the pr/eclipse/ref-point-migration/6/receiver-resource-selection branch from a8b8843 to 8ca868c Compare June 25, 2020 00:19
@tobous
Copy link
Member Author

tobous commented Jun 25, 2020

Rebased onto current base branch without any interaction.

@tobous tobous force-pushed the pr/eclipse/ref-point-migration/5/ui-internals-2 branch 2 times, most recently from 916f685 to 9c019ad Compare July 2, 2020 19:05
@tobous tobous force-pushed the pr/eclipse/ref-point-migration/6/receiver-resource-selection branch from 8ca868c to 7a0ee19 Compare July 2, 2020 19:07
@tobous
Copy link
Member Author

tobous commented Jul 2, 2020

Rebased onto current base branch without any interaction.

srossbach
srossbach previously approved these changes Jul 20, 2020
Copy link
Contributor

@srossbach srossbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too big to fail I guess. Likely that this patch has to be evaluated through real testing/usage of Saros.

@tobous tobous force-pushed the pr/eclipse/ref-point-migration/5/ui-internals-2 branch from 9c019ad to 06430a3 Compare August 4, 2020 03:34
@tobous tobous force-pushed the pr/eclipse/ref-point-migration/6/receiver-resource-selection branch from 7a0ee19 to 14d14ee Compare August 4, 2020 03:35
@tobous
Copy link
Member Author

tobous commented Aug 4, 2020

Rebased onto current base branch without any interactions.

@tobous
Copy link
Member Author

tobous commented Aug 4, 2020

Added commit to remove logic to preselect already shared reference points.

@tobous tobous force-pushed the pr/eclipse/ref-point-migration/5/ui-internals-2 branch from 06430a3 to c4b31eb Compare August 11, 2020 06:08
@tobous tobous force-pushed the pr/eclipse/ref-point-migration/6/receiver-resource-selection branch from 9cd5b81 to b379233 Compare August 11, 2020 06:09
@tobous
Copy link
Member Author

tobous commented Aug 11, 2020

Rebased onto current base branch without any interactions.

@tobous tobous force-pushed the pr/eclipse/ref-point-migration/5/ui-internals-2 branch from c4b31eb to 46cec54 Compare October 19, 2020 09:41
@tobous tobous force-pushed the pr/eclipse/ref-point-migration/6/receiver-resource-selection branch from b379233 to 08edea6 Compare October 19, 2020 09:41
@tobous
Copy link
Member Author

tobous commented Oct 19, 2020

Rebased onto current base branch without any interaction.

@tobous tobous force-pushed the pr/eclipse/ref-point-migration/5/ui-internals-2 branch from 46cec54 to b59d6c7 Compare October 21, 2020 18:30
Base automatically changed from pr/eclipse/ref-point-migration/5/ui-internals-2 to master October 21, 2020 18:34
@tobous tobous dismissed srossbach’s stale review October 21, 2020 18:34

The base branch was changed.

Reworks the project option composite to work with the new resource
model. To keep the changes as small as possible, the new handling is not
yet used. Instead, the old methods are marked as deprecated. Their usage
will be replaced in a separate commit reworking the EnterProjectNamePage
(the class containing all ProjectOptionComposites for all shared
reference points).

Adjusts the composite to have three option for the user to choose:
  * create a new project
  * create a new directory
  * use an existing directory

Adds a unified handling to select a specific option programmatically and
disable all other fields.

Adds separate methods to set a specific option/input of the dialog.

Adjusts the "browse" dialog to actually return the path of the chosen
resource instead of always returning the base project for the resource.
Replaces the text of the button with the Eclipse folder icon.

Adjusts the text shown in the composite to match the new content.

Adds ReferencePointOptionResult as a data holder class for the result of
the page. This makes it easier to pass the selected option to other
classes.
Renames ProjectOptionComposite to ReferencePointOptionComposite.

Renames ProjectOptionListener to ReferencePointOptionListener. Renames
the listener methods to match the new name. Adds javadoc.
Adds a comment to ignore the "Unused Property" warning in the
messages.properties files. This is necessary as IntelliJ does not detect
the fields as being used as they are injected through the use of
reflections.
Adds methods to obtain the container handle for the selection
represented by the reference point option result.

Adds IllegalInputException. The exception is used to signal that the
input represented by the result is not valid. The reason why the result
is invalid is passed as the exception message.

The added error messages will be displayed as an error in
EnterProjectNamePage, so it is important that they are concise and also
mention which reference point they belong to.
Rewrites EnterProjectNamePage to better match the new resource sharing
model and the new ReferencePointOptionComposite design.

Updates the validation logic to match the new input fields. Trims the
input strings to avoid issues where values were seen as different due to
leading or trailing spaces that are dropped during the resource
conversion, leading to resource clashes.

Updates the shown error messages to make it clearer which reference
point composite they belong to.

Updates the error message logic to always display the error message of
the currently selected tab (if present). This makes it much easier for
the user to determine what the issue with the current input actually is.
Furthermore, if the error messages are updated and there is no error for
the current tab, the error message is only changed if the currently
displayed message is no longer valid. This was done to avoid potentially
confusing the user be unnecessarily switching the error message even
though both messages are not related to the current tab.

Updates the preselection logic. The new logic now always selects the
best guess, even if it is not a valid selection. This approach was
chosen as it forces the user to manually deal with potential conflicts
instead of just giving an easy alternative. This should hopefully give
the user a better understanding of what the outcome of the resource
negotiation will be.
Updates the preselection logic to also suggest using an existing project
if a shared reference point has the same name. This heuristic was chosen
as project names should be somewhat unique in a normal project.
Furthermore, sharing entire projects should be a common use case, so
making it as easy to do so should increase the usability of the
preselection logic.

The preselection logic will also be extended in a future PR by using the
workspace relative path on the sender side to provide the basis for a
good guess on how to map the reference point locally.

Updates the text displayed in the composite, the variable and method
names, and the javadoc to match the new resource sharing model.
Adds checks to EnterProjectNamePage to ensure that the chosen local
representations will not lead to nested reference points.
Renames EnterProjectNamePage to LocalRepresentationSelectionPage to
better represent the purpose of the page.

Subsequently renames the entries in Messages and messages.properties.
Migrates AddProjectToSessionWizard to work with the new resource model
and more specifically with the new implementation of
LocalRepresentationSelectionPage and ReferencePointOptionComposite.

Upgrades the text shown to the user to represent the new resource
sharing model.
Removes unnecessary "final" declarations in the method context.

Removes unnecessary explicit argument types.

Replaces anonymous inner classes with lambda expressions.

Uses removeIf(...) to remove objects from a map. (Done automatically by
IntelliJ.)

Renames remaining local variables to match the new resource model.
Renames AddProjectToSessionWizard to AddReferencePointsToSessionWizard.

Subsequently renames the entries in Messages and messages.properties.

Adjusts reference to previous name in STF Constants. This is only a
hotfix to keep the STF test launch profiles from crashing. The proper
adjustment of the STF is done separately.
Removes the remaining deprecated methods remaining from the old UI
implementation in ReferencePointOptionComposite and
LocalRepresentationSelectionPage as they are no longer used.
Removes the logic to preselect the mapping for already shared reference
points from the local representation selection page.

The logic was necessary in cases where the mapping of a partial sharing
was expanded. With partial sharing no longer being supported, the logic
is no longer needed as the incoming resource negotiation should always
only contain non-shared reference points.
@tobous tobous force-pushed the pr/eclipse/ref-point-migration/6/receiver-resource-selection branch from 08edea6 to 075055f Compare October 21, 2020 18:35
@tobous
Copy link
Member Author

tobous commented Oct 21, 2020

Rebased onto current master without any interaction.

@saros-infrastructure
Copy link

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- eclipse/src/saros/ui/wizards/pages/LocalRepresentationSelectionPage.java  9
- eclipse/src/saros/ui/widgets/wizard/ReferencePointOptionResult.java  16
- eclipse/src/saros/exception/IllegalInputException.java  1
- eclipse/src/saros/ui/wizards/AddReferencePointsToSessionWizard.java  12
- eclipse/src/saros/ui/widgets/wizard/ReferencePointOptionComposite.java  5
         

See the complete overview on Codacy

@tobous tobous merged commit 0aadd5b into master Oct 21, 2020
@tobous tobous deleted the pr/eclipse/ref-point-migration/6/receiver-resource-selection branch October 21, 2020 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Eclipse Issue affecting Saros for Eclipse (Saros/E) Aspect: GUI Issues specific to the Saros GUI Breaks Compatibility Change breaking compatibility with previous releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants