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

[Refacto] Add delegate generic type and improve lazy loading extension code in preloading collection #464

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

antoinebhs
Copy link
Collaborator

@antoinebhs antoinebhs commented Oct 9, 2024

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines

What kind of change does this PR introduce?

  • Closes cleanup PreloadingNetworkStoreClient assumes its delegate is CachedNetworkStoreClient #420

    • Add delegate generic type in AbstractForwardingNetworkStoreClient. This explains that all NetworkStoreClient can't be used in any order (for example that a PreloadingNetworkStoreClient needs to be backed by a CachedNetworkStoreClient to work). This was already the case in the different client constructors, for example here:
      public PreloadingNetworkStoreClient(CachedNetworkStoreClient delegate, boolean allCollectionsNeededForBusView,
      ExecutorService executorService) {
      super(delegate);
      this.allCollectionsNeededForBusView = allCollectionsNeededForBusView;
      this.executorService = Objects.requireNonNull(executorService);
      }
    • In PreloadingNetworkStoreClient, there was some logic to know if the resourceType is cached. This logic is already implemented in the CachedNetworkStoreClient (and especially in CollectionCache). I removed this logic and maintain the code only to know if all collections needed for bus view have been loaded in the client. It avoids calling loadAllCollectionsNeededForBusView if it's not necessary.
    • As specified in the issue, I avoid to call the delegate.getGenerators() twice in the method getGenerators() if we are not in ALL_COLLECTION_NEED_FOR_BUS_VIEW (and same for all types of equipments). Now, the delegate is only called once in this case. In the case where we are in ALL_COLLECTION_NEED_FOR_BUS_VIEW, we first load all the collections and then retrieve the generator collection.
  • Code improvements:

    • getAllExtensionsAttributesByResourceType and getAllExtensionsAttributesByResourceTypeAndExtensionName() are now only used to fetch resources from the server (implemented only in RestNetworkStoreClient).
    • there are new methods in CachedNetworkStoreClient (method load*) that only load extension attributes to cache but does not return anything (the returns were not used anyway) it fixes the TODO from Fix looking on all resources when retrieving extensions for preloading collection #463
    • some tests improvements or fixes because some tests were using the return of the methods that only load extension attributes to cache
    • I added some tests to cover method in PreloadingNetworkStoreClient as it was poorly covered. As an example, I took the groundCache test which was quite complete. It's not extensive but better than no tests.

This PR does not change any feature, it's only about code refactoring.

@antoinebhs antoinebhs force-pushed the refacto-preloading-collection-lazy-extension branch 2 times, most recently from 195201d to 2cd5baf Compare October 9, 2024 16:03
Signed-off-by: BOUHOURS Antoine <[email protected]>
@antoinebhs antoinebhs force-pushed the refacto-preloading-collection-lazy-extension branch from 2cd5baf to e099d49 Compare October 9, 2024 16:17
Signed-off-by: BOUHOURS Antoine <[email protected]>
@antoinebhs antoinebhs changed the title Refactor lazy loading of extension in preloading collection [WIP] Refactor lazy loading of extension in preloading collection Oct 16, 2024
… into refacto-preloading-collection-lazy-extension
Signed-off-by: BOUHOURS Antoine <[email protected]>
Signed-off-by: BOUHOURS Antoine <[email protected]>
@antoinebhs antoinebhs force-pushed the refacto-preloading-collection-lazy-extension branch from e2f0202 to 10ed186 Compare October 17, 2024 15:49
@antoinebhs antoinebhs force-pushed the refacto-preloading-collection-lazy-extension branch from e50a021 to 935b96b Compare October 18, 2024 09:18
Copy link

sonarcloud bot commented Oct 18, 2024

@antoinebhs antoinebhs changed the title [WIP] Refactor lazy loading of extension in preloading collection [Refacto] Add delegate generic type and improve lazy loading extension code in preloading collection Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cleanup PreloadingNetworkStoreClient assumes its delegate is CachedNetworkStoreClient
1 participant