Elide 5.x dynamic config standa 8000 lone #1259

AvaniMakwana · 2020-04-10T15:51:04Z

moizarafat · 2020-04-22T13:57:54Z

elide-standalone/src/main/java/com/yahoo/elide/standalone/Util.java

@@ -53,28 +64,61 @@ public static EntityManagerFactory getEntityManagerFactory(String modelPackageNa
            options.put("javax.persistence.jdbc.password", "elide123");
        }

-        PersistenceUnitInfo persistenceUnitInfo = new PersistenceUnitInfoImpl("elide-stand-alone",
-                combineModelEntities(modelPackageName, includeAsyncModel), options);
+        dynamicEntityCompiler = new ElideDynamicEntityCompiler(dynamicConfigPath);


This line should be inside the if block at L-68, otherwise even when the dynamic config is disabled it will try to generate the Config POJOs.

moizarafat · 2020-04-22T14:29:19Z

elide-standalone/src/main/java/com/yahoo/elide/standalone/Util.java

+            ElideDynamicEntityCompiler.bindClasses.addAll(populateBindClasses(dynamicEntityCompiler,
+                    FromSubquery.class));
+            // add classes
+            ElideDynamicEntityCompiler.bindClasses.addAll(ClassScanner.getAnnotatedClasses(Entity.class));


These 3 sets of classes should not be binded to ElideDynamicEntityCompiler.bindClasses as they are not compiled through dynamic compiler. These 3 set of classes should be put in options as AvailableSettings.LOADED_CLASSES, <ClassScanner Scanned classes> . Refer

elide/elide-spring/elide-spring-boot-autoconfigure/src/main/java/com/yahoo/elide/spring/config/ElideDynamicConfiguration.java

Line 97 in 2d6436d

puiPropertyMap.put(AvailableSettings.LOADED_CLASSES, bindClasses);

moizarafat · 2020-04-22T15:48:50Z

elide-standalone/src/main/java/com/yahoo/elide/standalone/Util.java


+        } catch (ClassNotFoundException e) {
+            // TODO Auto-generated catch block
+            e.printStackTrace();


We should probably not catch the exception and let the error propagate.

moizarafat · 2020-04-22T15:57:01Z

elide-standalone/src/main/java/com/yahoo/elide/standalone/Util.java

+        Set<Class> bindClasses = new HashSet<>();
+        List<String> dynamicClasses = ElideDynamicEntityCompiler.classNames;
+        for (String dynamicClass : dynamicClasses) {
+            Class<?> bindClass = compiler.getClassLoader().loadClass(dynamicClass).getClass();


We dont need to do loadClass.getClass. Only loadClass is needed.

moizarafat · 2020-04-22T16:01:08Z

elide-standalone/src/main/java/com/yahoo/elide/standalone/Util.java

+    @SuppressWarnings({ "unchecked", "rawtypes" })
+    public static List<String> findAnnotatedClasses(ElideDynamicEntityCompiler compiler, Class annotationClass)
+            throws ClassNotFoundException {
+        List<String> annotatedClass = new ArrayList<String>();


annotatedClasses instead of annotatedClass

moizarafat · 2020-04-22T16:18:54Z

elide-standalone/src/main/java/com/yahoo/elide/standalone/config/ElideResourceConfig.java

@@ -49,24 +51,36 @@

    private static MetricRegistry metricRegistry = null;
    private static HealthCheckRegistry healthCheckRegistry = null;
+    private ElideDynamicEntityCompiler dynamicEntityCompiler = null;


This is already defined as static in Util. We should reuse from there.

moizarafat · 2020-04-22T16:24:00Z

elide-standalone/src/main/java/com/yahoo/elide/standalone/config/ElideResourceConfig.java

     */
    @Inject
    public ElideResourceConfig(ServiceLocator injector, @Context ServletContext servletContext) {
        this.injector = injector;

        settings = (ElideStandaloneSettings) servletContext.getAttribute(ELIDE_STANDALONE_SETTINGS_ATTR);

+        dynamicEntityCompiler = new ElideDynamicEntityCompiler(settings.getDynamicConfigPath());


This piece of code is duplicated between Util.getEntityManager and here. Instead this could be a separate method in Util like initialize dynamic compiler which takes a boolean argument derived from enableDynamicModelConfig . And based on the boolean value and if dynamicEntityCompiler is null, dynamicEntityCompiler will be initialized and classes compiled.

moizarafat · 2020-04-22T16:30:58Z

elide-standalone/src/main/java/com/yahoo/elide/standalone/config/ElideResourceConfig.java

+
+                        Swagger swagger = builder.build().basePath(dynamicBasePath);
+
+                        swaggerDocs.put("dynamic model", swagger);


lets have a singular name instead of with spaces. Just "dynamic" instead of "dynamic model";

moizarafat · 2020-04-22T16:31:47Z

elide-standalone/src/main/java/com/yahoo/elide/standalone/config/ElideResourceConfig.java

+                        SwaggerBuilder builder = new SwaggerBuilder(dictionary, info);
+
+                        //Default value of getJsonApiPathSpec() ends with /* at the end. need to remove.
+                        String dynamicBasePath = settings.getJsonApiPathSpec().replaceAll("/\\*", "");


Since this is logic is same between both blocks, we can get it out in a separate common variable.

moizarafat · 2020-04-22T17:12:42Z

...lone/src/main/java/com/yahoo/elide/standalone/dynamic/config/ElideDynamicEntityCompiler.java

+     * @param name - class name
+     * @return compiled class
+     */
+    public Class<?> getCompiled(String name) {


This method is not used anywhere. we should remove it.

elide-standalone/src/test/java/com/yahoo/elide/standalone/ElideStandaloneTest.java

moizarafat

moizarafat · 2020-04-23T05:46:32Z

elide-standalone/src/main/java/com/yahoo/elide/standalone/Util.java

+    }
+
+    @SuppressWarnings({ "unchecked", "rawtypes" })
+    public static Set<Class> populateBindClasses(ElideDynamicEntityCompiler compiler,


We can rename this to findAnnotatedClasses.

moizarafat · 2020-04-23T05:50:27Z

elide-standalone/src/main/java/com/yahoo/elide/standalone/Util.java

        if (includeAsyncModel) {
            modelEntities.addAll(getAllEntities(AsyncQuery.class.getPackage().getName()));
        }
+        if (includeDynamicModel) {
+            modelEntities.addAll(findAnnotatedClasses(compiler, Entity.class));


We need to add classes of type FromTable, FromSubQuery

elide-standalone/src/main/java/com/yahoo/elide/standalone/Util.java

elide-standalone/pom.xml

elide-standalone/src/test/java/com/yahoo/elide/standalone/ElideStandaloneTest.java

elide-standalone/src/test/resources/models/variables.hjson

elide-standalone/src/main/java/com/yahoo/elide/standalone/config/ElideResourceConfig.java

aklish · 2020-05-11T21:19:36Z

elide-standalone/src/main/java/com/yahoo/elide/standalone/config/ElideResourceConfig.java

                        swaggerDocs.put("async", swagger);
                    }
+                    // bind dynamic models
+                    if (settings.enableDynamicModelConfig()) {
+                        EntityDictionary dictionary = new EntityDictionary(new HashMap<>());


The swagger builder doesn't need a separate dictionary to only build a subset of models. YOu can provide it the list of classes to load. That's the recommended approach. Let's follow that.

elide-standalone/src/main/java/com/yahoo/elide/standalone/config/ElideResourceConfig.java

coveralls · 2020-05-17T01:09:37Z

elide-standalone/src/test/resources/models/tables/PostView.hjson

elide-standalone/src/test/java/example/ElideStandaloneTest.java

elide-standalone/src/main/java/com/yahoo/elide/standalone/config/ElideStandaloneSettings.java

elide-standalone/src/main/java/com/yahoo/elide/standalone/config/ElideResourceConfig.java

AvaniMakwana added work in progress DO NOT MERGE labels Apr 10, 2020

AvaniMakwana force-pushed the elide-5.x-dynamic-config-standalone-integration branch from 08c36c6 to 9ac9208 Compare April 16, 2020 05:52

moizarafat reviewed Apr 22, 2020

View reviewed changes

elide-standalone/src/test/java/com/yahoo/elide/standalone/ElideStandaloneTest.java Outdated Show resolved Hide resolved

moizarafat requested changes Apr 22, 2020

View reviewed changes

moizarafat reviewed Apr 23, 2020

View reviewed changes

elide-standalone/src/main/java/com/yahoo/elide/standalone/Util.java Outdated Show resolved Hide resolved

moizarafat force-pushed the elide-5.x-dynamic-config-integration branch from cc579ef to 8320245 Compare April 30, 2020 23:40

moizarafat changed the base branch from elide-5.x-dynamic-config-integration to elide-5.x-async May 4, 2020 14:32

moizarafat changed the base branch from elide-5.x-async to elide-5.x-dynamic-config-integration 1E0A May 4, 2020 14:33

moizarafat removed DO NOT MERGE work in progress labels May 11, 2020

aklish requested changes May 11, 2020

View reviewed changes

moizarafat changed the base branch from elide-5.x-dynamic-config-integration to master May 13, 2020 05:04

moizarafat changed the base branch from master to elide-5.x-dynamic-config-integration May 13, 2020 05:04

moizarafat changed the base branch from elide-5.x-dynamic-config-integration to AddAggregationDataStoreDocs May 13, 2020 05:34

moizarafat changed the base branch from AddAggregationDataStoreDocs to elide-5.x-dynamic-config-integration May 13, 2020 05:35

moizarafat changed the base branch from elide-5.x-dynamic-config-integration to master May 13, 2020 05:36

moizarafat changed the base branch from master to elide-5.x-dynamic-config-integration May 13, 2020 05:36

moizarafat changed the base branch from elide-5.x to master May 19, 2020 17:18

moizarafat changed the base branch from master to elide-5.x May 19, 2020 17:18

moizarafat force-pushed the elide-5.x-dynamic-config-standalone-integration branch 2 times, most recently from fa212e9 to 1b32fcb Compare May 19, 2020 19:59

moizarafat and others added 2 commits May 21, 2020 14:43

Fix method call, Swagger Doc Update

6cb1de1

Co-authored-by: AvaniMakwana <Avani.makwana@outlook.com>

Swagger doc update

fc96a13

Co-authored-by: AvaniMakwana <Avani.makwana@outlook.com>

moizarafat force-pushed the elide-5.x-dynamic-config-standalone-integration branch from 79b328b to fc96a13 Compare May 21, 2020 18:44

update pom.xml

b64ac72

Co-authored-by: AvaniMakwana <Avani.makwana@outlook.com>

moizarafat force-pushed the elide-5.x-dynamic-config-standalone-integration branch from 4fec8b7 to b64ac72 Compare May 21, 2020 18:46

update pom.xml

b9ebd6d

Co-authored-by: AvaniMakwana <Avani.makwana@outlook.com>

aklish requested changes May 21, 2020

View reviewed changes

moizarafat and others added 4 commits May 21, 2020 18:19

review comments

7249fdb

Co-authored-by: AvaniMakwana <Avani.makwana@outlook.com>

Update ElideStandaloneSettings.java

c81eb87

Update pom.xml

864e02d

Update pom.xml

7b71404

aklish approved these changes May 26, 2020

View reviewed changes

aklish merged commit a2e1ef3 into elide-5.x May 26, 2020

aklish deleted the elide-5.x-dynamic-config-standalone-integration branch May 26, 2020 19:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Elide 5.x dynamic config standa 8000 lone #1259

Elide 5.x dynamic config standalone #1259

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Elide 5.x dynamic config standa 8000 lone #1259

Elide 5.x dynamic config standalone #1259

Uh oh!

Conversation

Description

Motivation and Context

How Has This Been Tested?

License

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!