Skip to content

Commit

Permalink
Remove defaults and limits from GenericStub and replace with user-spe…
Browse files Browse the repository at this point in the history
…cific parser

Co-authored-by: Veselin Nikolov <[email protected]>
  • Loading branch information
IoannisPanagiotas and vnickolov committed Sep 2, 2024
1 parent 6c93e16 commit 83e8cef
Show file tree
Hide file tree
Showing 60 changed files with 71 additions and 148 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ private static NodePropertyStepFactoryUsingStubs create() {
// not great, one day these should be injected
var configurationParser = new ConfigurationParser(DefaultsConfiguration.Instance, LimitsConfiguration.Instance);
var validationService = new ValidationService(
DefaultsConfiguration.Instance,
LimitsConfiguration.Instance,
configurationParser
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@
*/
package org.neo4j.gds.ml.pipeline;

import org.neo4j.gds.api.User;
import org.neo4j.gds.applications.algorithms.metadata.Algorithm;
import org.neo4j.gds.config.AlgoBaseConfig;
import org.neo4j.gds.configuration.DefaultsConfiguration;
import org.neo4j.gds.configuration.LimitsConfiguration;
import org.neo4j.gds.core.CypherMapWrapper;
import org.neo4j.gds.core.Username;
import org.neo4j.gds.procedures.algorithms.configuration.ConfigurationParser;
Expand All @@ -33,17 +32,12 @@
class ValidationService {
private final ConfigurationParsersForMutateMode configurationParsersForMutateMode = new ConfigurationParsersForMutateMode();

private final DefaultsConfiguration defaultsConfiguration;
private final LimitsConfiguration limitsConfiguration;
private final ConfigurationParser configurationParser;

ValidationService(
DefaultsConfiguration defaultsConfiguration,
LimitsConfiguration limitsConfiguration,
ConfigurationParser configurationParser
) {
this.defaultsConfiguration = defaultsConfiguration;
this.limitsConfiguration = limitsConfiguration;

this.configurationParser = configurationParser;
}

Expand All @@ -67,11 +61,9 @@ private <CONFIGURATION extends AlgoBaseConfig> void validateAnonymously(
Map<String, Object> configuration
) {
configurationParser.parseConfiguration(
defaultsConfiguration,
limitsConfiguration,
Username.EMPTY_USERNAME.username(),
configuration,
(__, cmw) -> parser.apply(cmw)
parser,
new User(Username.EMPTY_USERNAME.username(),false)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -500,11 +500,9 @@ private GraphDataScienceProcedures constructFacade() {
.build();


var configurationParser = new ConfigurationParser(DefaultsConfiguration.Instance, LimitsConfiguration.Instance);
var configurationParser = new UserSpecificConfigurationParser(new ConfigurationParser(DefaultsConfiguration.Instance, LimitsConfiguration.Instance),requestScopedDependencies.getUser());

var genericStub = GenericStub.create(
DefaultsConfiguration.Instance,
LimitsConfiguration.Instance,
graphStoreCatalogService,
configurationParser,
requestScopedDependencies
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -499,17 +499,17 @@ private GraphDataScienceProcedures createFacade() {
var logMock = mock(org.neo4j.gds.logging.Log.class);

final GraphStoreCatalogService graphStoreCatalogService = new GraphStoreCatalogService();
var configurationParser = new ConfigurationParser(DefaultsConfiguration.Instance, LimitsConfiguration.Instance);
var requestScopedDependencies = RequestScopedDependencies.builder()
.with(DatabaseId.of(db.databaseName()))
.with(TaskRegistryFactory.empty())
.with(TerminationFlag.RUNNING_TRUE)
.with(new User(getUsername(), false))
.with(EmptyUserLogRegistryFactory.INSTANCE)
.build();

var configurationParser = new UserSpecificConfigurationParser(new ConfigurationParser(DefaultsConfiguration.Instance, LimitsConfiguration.Instance),requestScopedDependencies.getUser());

var genericStub = GenericStub.create(
DefaultsConfiguration.Instance,
LimitsConfiguration.Instance,
graphStoreCatalogService,
configurationParser,
requestScopedDependencies
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -602,10 +602,10 @@ private GraphDataScienceProcedures constructGraphDataScienceProcedures() {
null,
null
);
var configurationParser = new ConfigurationParser(DefaultsConfiguration.Instance, LimitsConfiguration.Instance);

var configurationParser = new UserSpecificConfigurationParser(new ConfigurationParser(DefaultsConfiguration.Instance, LimitsConfiguration.Instance),requestScopedDependencies.getUser());

var genericStub = GenericStub.create(
DefaultsConfiguration.Instance,
LimitsConfiguration.Instance,
graphStoreCatalogService,
configurationParser,
requestScopedDependencies
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.neo4j.gds.api.DatabaseId;
import org.neo4j.gds.api.DefaultValue;
import org.neo4j.gds.api.GraphStore;
import org.neo4j.gds.api.User;
import org.neo4j.gds.api.schema.GraphSchema;
import org.neo4j.gds.applications.ApplicationsFacade;
import org.neo4j.gds.applications.algorithms.machinery.RequestScopedDependencies;
Expand Down Expand Up @@ -63,6 +64,7 @@
import org.neo4j.gds.procedures.algorithms.AlgorithmsProcedureFacade;
import org.neo4j.gds.procedures.algorithms.centrality.CentralityProcedureFacade;
import org.neo4j.gds.procedures.algorithms.configuration.ConfigurationParser;
import org.neo4j.gds.procedures.algorithms.configuration.UserSpecificConfigurationParser;
import org.neo4j.gds.procedures.algorithms.stubs.GenericStub;
import org.neo4j.gds.termination.TerminationFlag;
import org.neo4j.gds.test.TestProc;
Expand Down Expand Up @@ -428,7 +430,9 @@ void shouldEstimateMemory() {
private static AlgorithmsProcedureFacade createAlgorithmsProcedureFacade() {
var requestScopedDependencies = RequestScopedDependencies.builder()
.with(TerminationFlag.RUNNING_TRUE)
.with(User.DEFAULT)
.build();

var applicationsFacade = ApplicationsFacade.create(
null,
null,
Expand All @@ -447,8 +451,8 @@ private static AlgorithmsProcedureFacade createAlgorithmsProcedureFacade() {
null,
null
);
var configurationParser = new ConfigurationParser(null, null);
var genericStub = GenericStub.create(null, null, null, configurationParser, requestScopedDependencies);
var configurationParser = new UserSpecificConfigurationParser(new ConfigurationParser(null, null),requestScopedDependencies.getUser());
var genericStub = GenericStub.create(null, configurationParser, requestScopedDependencies);
var centralityProcedureFacade = CentralityProcedureFacade.create(
genericStub,
applicationsFacade,
Expand Down
4 changes: 1 addition & 3 deletions proc/test/src/main/java/org/neo4j/gds/ProcedureRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
import org.neo4j.gds.metrics.MetricsFacade;
import org.neo4j.gds.metrics.procedures.DeprecatedProceduresMetricService;
import org.neo4j.gds.procedures.AlgorithmProcedureFacadeBuilderFactory;
import org.neo4j.gds.procedures.GraphCatalogProcedureFacadeFactory;
import org.neo4j.gds.procedures.DatabaseIdAccessor;
import org.neo4j.gds.procedures.GraphCatalogProcedureFacadeFactory;
import org.neo4j.gds.procedures.GraphDataScienceProcedures;
import org.neo4j.gds.procedures.ProcedureCallContextReturnColumns;
import org.neo4j.graphdb.GraphDatabaseService;
Expand Down Expand Up @@ -170,8 +170,6 @@ private static GraphDataScienceProcedures createGraphDataScienceProcedures(
var modelCatalog = new OpenModelCatalog();

var algorithmFacadeBuilderFactory = new AlgorithmProcedureFacadeBuilderFactory(
DefaultsConfiguration.Instance,
LimitsConfiguration.Instance,
graphStoreCatalogService
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ public ArticulationPointsMutateConfig parseConfiguration(Map<String, Object> con
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
ArticulationPointsMutateConfig::of,
(config) -> estimationModeBusinessFacade.articulationPoints()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ public ClosenessCentralityMutateConfig parseConfiguration(Map<String, Object> co
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
ClosenessCentralityMutateConfig::of,
estimationModeBusinessFacade::closenessCentrality
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ public BetweennessCentralityMutateConfig parseConfiguration(Map<String, Object>
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
BetweennessCentralityMutateConfig::of,
estimationModeBusinessFacade::betweennessCentrality
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ public InfluenceMaximizationMutateConfig parseConfiguration(Map<String, Object>
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
InfluenceMaximizationMutateConfig::of,
estimationModeBusinessFacade::celf
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ public ClosenessCentralityMutateConfig parseConfiguration(Map<String, Object> co
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
ClosenessCentralityMutateConfig::of,
estimationModeBusinessFacade::closenessCentrality
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ public DegreeCentralityMutateConfig parseConfiguration(Map<String, Object> confi
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
DegreeCentralityMutateConfig::of,
estimationModeBusinessFacade::degreeCentrality
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ public HarmonicCentralityMutateConfig parseConfiguration(Map<String, Object> con
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
HarmonicCentralityMutateConfig::of,
__ -> estimationModeBusinessFacade.harmonicCentrality()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ public PageRankMutateConfig parseConfiguration(Map<String, Object> configuration
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
configProducer,
__ -> estimationModeBusinessFacade.pageRank()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ public ApproxMaxKCutMutateConfig parseConfiguration(Map<String, Object> configur
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
ApproxMaxKCutMutateConfig::of,
estimationModeBusinessFacade::approximateMaximumKCut
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ public K1ColoringMutateConfig parseConfiguration(Map<String, Object> configurati
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
K1ColoringMutateConfig::of,
__ -> estimationModeBusinessFacade.k1Coloring()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ public KCoreDecompositionMutateConfig parseConfiguration(Map<String, Object> con
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
KCoreDecompositionMutateConfig::of,
__ -> estimationModeBusinessFacade.kCore()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ public KmeansMutateConfig parseConfiguration(Map<String, Object> configuration)
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> rawConfiguration) {
return genericStub.getMemoryEstimation(
username,
rawConfiguration,
KmeansMutateConfig::of,
estimationModeBusinessFacade::kMeans
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ public LabelPropagationMutateConfig parseConfiguration(Map<String, Object> confi
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> rawConfiguration) {
return genericStub.getMemoryEstimation(
username,
rawConfiguration,
LabelPropagationMutateConfig::of,
__ -> estimationModeBusinessFacade.labelPropagation()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ public LocalClusteringCoefficientMutateConfig parseConfiguration(Map<String, Obj
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
LocalClusteringCoefficientMutateConfig::of,
estimationModeBusinessFacade::lcc
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ public LeidenMutateConfig parseConfiguration(Map<String, Object> configuration)
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
LeidenMutateConfig::of,
estimationModeBusinessFacade::leiden
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ public LouvainMutateConfig parseConfiguration(Map<String, Object> configuration)
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
LouvainMutateConfig::of,
estimationModeBusinessFacade::louvain
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ public ModularityOptimizationMutateConfig parseConfiguration(Map<String, Object>
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
ModularityOptimizationMutateConfig::of,
__ -> estimationModeBusinessFacade.modularityOptimization()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ public SccMutateConfig parseConfiguration(Map<String, Object> configuration) {
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
SccMutateConfig::of,
__ -> estimationModeBusinessFacade.scc()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ public TriangleCountMutateConfig parseConfiguration(Map<String, Object> configur
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
TriangleCountMutateConfig::of,
__ -> estimationModeBusinessFacade.triangleCount()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ public WccMutateConfig parseConfiguration(Map<String, Object> configuration) {
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> configuration) {
return genericStub.getMemoryEstimation(
username,
configuration,
WccMutateConfig::of,
estimationModeBusinessFacade::wcc
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,18 @@ <CONFIG extends AlgoBaseConfig> CONFIG parseConfiguration(
return parseConfiguration(defaults, limits, user.getUsername(), configuration, lexer);
}

public <CONFIG extends AlgoBaseConfig> CONFIG parseConfigurationWithoutDefaultsAndLimits(
Map<String, Object> configuration,
BiFunction<String, CypherMapWrapper, CONFIG> lexer,
String username
) {
return parseConfiguration(DefaultsConfiguration.Empty, LimitsConfiguration.Empty, username, configuration, lexer);
}

/**
* Configuration parsing using directly configured defaults and limits
*/
public <CONFIG extends AlgoBaseConfig> CONFIG parseConfiguration(
private <CONFIG extends AlgoBaseConfig> CONFIG parseConfiguration(
DefaultsConfiguration defaultsConfiguration,
LimitsConfiguration limitsConfiguration,
String username,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,15 @@ public <CONFIG extends AlgoBaseConfig> CONFIG parseConfiguration(
user
);
}

public <CONFIG extends AlgoBaseConfig> CONFIG parseConfigurationWithoutDefaultsAndLimits(
Map<String, Object> configuration,
Function<CypherMapWrapper, CONFIG> lexer
) {
return configurationParser.parseConfigurationWithoutDefaultsAndLimits(
configuration,
(__, cypherMapWrapper) -> lexer.apply(cypherMapWrapper),
user.getUsername()
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ public FastRPMutateConfig parseConfiguration(Map<String, Object> configuration)
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> rawConfiguration) {
return genericStub.getMemoryEstimation(
username,
rawConfiguration,
FastRPMutateConfig::of,
estimationModeBusinessFacade::fastRP
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ public GraphSageMutateConfig parseConfiguration(Map<String, Object> configuratio
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> rawConfiguration) {
return genericStub.getMemoryEstimation(
username,
rawConfiguration,
cypherMapWrapper -> GraphSageMutateConfig.of(username, cypherMapWrapper),
configuration -> estimationModeBusinessFacade.graphSage(configuration, true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ public HashGNNMutateConfig parseConfiguration(Map<String, Object> configuration)
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> rawConfiguration) {
return genericStub.getMemoryEstimation(
username,
rawConfiguration,
HashGNNMutateConfig::of,
estimationModeBusinessFacade::hashGnn
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ public Node2VecMutateConfig parseConfiguration(Map<String, Object> configuration
@Override
public MemoryEstimation getMemoryEstimation(String username, Map<String, Object> rawConfiguration) {
return genericStub.getMemoryEstimation(
username,
rawConfiguration,
Node2VecMutateConfig::of,
estimationModeBusinessFacade::node2Vec
Expand Down
Loading

0 comments on commit 83e8cef

Please sign in to comment.