Support stable prioritization of MinedConditions

[HannesWell]

In situations where multiple mined-conditions are available to satisfy a requirement it would be very convenient if there were means to prioritize certain conditions over others. From my observation it is currently random which condition is selected (i.e. depends on the order of elements in a HashMap/-Set).

For example we are using a custom access-cycle configuration that uses two LocalConditions and one RemoteConditions miner, similar like the FocusedAccessCycleConfiguration.Wide configuration does, so this issue probably also applies to all that use that config.
When a user has a operational floating server available plus a valid local personal license, it would be good if the Access-Cyle would always select the personal and acquires the corresponding grant over the floating-license.
This reduces the number of occupied floating seats without the need to disable the floating license locally.

At the moment I can think of two ways to implement this:

• Ensure the miner registries establish a stable order (e.g. via LinkedHashMap to maintain the order of the passed List) to get a stable condition order in the following process.
  The priorities are then implicitly defined in the AccessCycleConfiguration through the order of the condition miners passed to the registry.
• Assign priorities to MinedConditions according to which the mined valid conditions are sorted before one is finally selected to satisfy a requirement
  The priorities are then defined explicitly by assigning an integer priority to each condition.

The first approach seems to be a bit simpler to implement to me but has the disadvantage that it the priorities are not explicitly modeled and that it has to be ensured that the order is also maintained in the entire mining process.

If you are interested I can offer to provide a PR to implement this in the way you prefer.

[eparovyshnaya]

Dear @HannesWell
thank you very much for the idea.

It needs to say, that actual products never licensed both ways simultaneousely. Wide configuration is illustrative for the products, which do not yet know, how exactly they are going to be licensed, as it is mostly the decision to be made not by development, but by management.

Been that said, the feature looks worthful.

Adding priority property to MinedConditions interface is not an option now. It is mature interface and it's worth lots of thinking to add anything to it.

But paying attention to the order of services in a registry can be a way.

Till this moment we declared that order of services is not relyable.
If now we pronounce order matters for conditions, for archtectural consistency we msut do the same for all the services for which interfaces multiple implementations can be supplied.

It means that

• AccessCycle implementation must be changed significantly
• all the access cycle services invocations must be revised from this point of view, reimplemented
• exhaustive testing must be supplied.

Any change to AccessCycle logic mean major version increment, so we plan it for 3.0.0.

[HannesWell]

Thank you @eparovyshnaya for your assessment and I'm glad you like the idea.

For now I found a simple workaround, using the JointRegistry, to create a registry that maintains a stable service order. Actually one could also use a custom implementation of the Registry interface:

	private static <I extends ServiceId, S extends Service<I>> Registry<I, S> createRegistry(List<S> services) {
		// Workaround to have a registry that maintains a stable service order, unlike ReadOnlyRegistry
		List<Registry<I, S>> regs = services.stream().map(ReadOnlyRegistry::new).collect(Collectors.toList());
		return new JointRegistry<>(regs);
	}

However this works well actually, the only problem that prevents me from using the described feature now, is is that in the end of the condition mining the stable order is lost because org.eclipse.passage.lic.base.FeatureFilter and org.eclipse.passage.lic.base.conditions.FeatureConditionPackuse an ordinary HashSet (i.e. Collectors.toSet()) to collect the elements that pass the filter.

It should be sufficient to change that collector to one that ensures stable ordering, for example Collectors.toList() or Collectors.toCollection(LinkedHashSet::new). The method signature only specifies Collection, so both are suitable from that perspective. From the behavioural point of view, I think changing those filters to maintain the input order is not harmful. Those filters don't specify that they are shuffling, actually I would have even expected the opposite, that a filter maintains the input order and just maybe does let some elements pass.

Are you willing to accept a PR that in both cases changes the Collectors.toSet() to Collectors.toList() respectively Collectors.toCollection(LinkedHashSet::new)? If so, I suspect you used toSet() for a reason and therefore in favour of the second option.

[eparovyshnaya]

Dear @HannesWell
thank you for the offered quick-fix,
but we rather interested in steady and consistent evolution.

As this particular case brings no crucial benefit in practice (it hardly appears in real life), there is no need for quick fixes.

To implement general service ordering feature, it requires some rethinking of major part of Passage Core. It also demands public interfaces changes and Access Cycle (main part of Passage) logic change, both can happen only in major release.

[HannesWell]

Dear @eparovyshnaya,
I understand that a general service ordering feature requires major work and changes that can only be done for a major release.

All I'm asking for now is to accept a PR to change the stream pipelines in org.eclipse.passage.lic.base.FeatureFilter and org.eclipse.passage.lic.base.conditions.FeatureConditionPack to use Collectors.toCollection(LinkedHashSet::new) instead of Collectors.toSet()).
Please tell me if I'm wrong, but from my understanding it is not a requirement for those filters to shuffle the elements and should therefore not be harmful for Passage.

I understand that this is not crucial for you, but I would appreciate if that small change is implemented and therefore offer to do the work and create a PR. As far as I can tell that should be totally sufficient to make it work in in conjunction with a custom stable-order Registry. I'm totally fine to relay on a workaround for now.

[eparovyshnaya]

Dear @HannesWell, it is merged.
Thank you for making it concise and literally single-word fix.

[HannesWell]

Dear @HannesWell, it is merged. Thank you for making it concise and literally single-word fix.

Dear @eparovyshnaya you are very welcome and thanks a lot!
I just tested it with a self build snapshot and in conjunction with the workaround described above it now worked well for me.

Btw. passage does not publish snapshot repos, only milestones, does it?

[eparovyshnaya]

@HannesWell
you can use integration latest p2 repo for yout target platform, if you want to have recent changes.

[HannesWell]

Thank you @eparovyshnaya, I obviously oversaw that when searching Passage's repos.