Extract out IdGeneratorBase and Add NonceGenerator #43

CyberDrudge · 2024-09-23T19:19:51Z

No description provided.

…hted-id-generation-1

santanusinha

Class designs need to be redone.

...r-discovery-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/GenerationResult.java

santanusinha · 2024-09-30T10:00:29Z

...ry-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/generator/IdGeneratorBase.java

+@Slf4j
+public class IdGeneratorBase {
+
+    private final int MINIMUM_ID_LENGTH;


santanusinha · 2024-09-30T10:03:26Z

...ry-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/generator/IdGeneratorBase.java

+public class IdGeneratorBase {
+
+    private final int MINIMUM_ID_LENGTH;
+    private final Pattern PATTERN;


santanusinha · 2024-09-30T10:04:01Z

...ry-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/generator/IdGeneratorBase.java

+    private final Pattern PATTERN;
+    @Getter
+    private static int NODE_ID;
+    protected final DateTimeFormatter DATE_TIME_FORMATTER;


don't use this naming pattern for non static

santanusinha · 2024-09-30T10:07:02Z

...ry-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/generator/IdGeneratorBase.java

+    }
+
+    public synchronized void registerGlobalConstraints(final IdValidationConstraint... constraints) {
+        registerGlobalConstraints(ImmutableList.copyOf(constraints));


List.copyOf()

...ry-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/generator/IdGeneratorBase.java

santanusinha · 2024-09-30T11:48:48Z

...y-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/nonce/RandomNonceGenerator.java

+
+    public RandomNonceGenerator(final IdFormatter idFormatter) {
+        super(idFormatter);
+        RetryPolicy<GenerationResult> RETRY_POLICY = RetryPolicy.<GenerationResult>builder()


These things should all be moved to base. It needs to expose a very limited set of methods (one or two) to generate the id based on logic(random in this case)

santanusinha · 2024-09-30T11:51:57Z

...y-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/nonce/RandomNonceGenerator.java

+    }
+
+    @Override
+    public DateTime getDateTimeFromTime(final long time) {


what is this method, why is this public and why is it placed below private methods?

santanusinha · 2024-09-30T11:52:38Z

...r-discovery-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/GenerationResult.java

+import lombok.Getter;
+
+@Getter
+@Builder


Remove builder here. All members should be populated.

santanusinha · 2024-09-30T11:53:31Z

ranger-discovery-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/IdGenerator.java

-
-    private static final List<IdValidationConstraint> GLOBAL_CONSTRAINTS = new ArrayList<>();
-    private static int nodeId;
+    private static final IdGeneratorBase baseGenerator = new IdGeneratorBase(


Too many comments already Havent reviewed this or any further file after this,

...ry-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/generator/IdGeneratorBase.java

r0goyal · 2024-12-29T16:38:48Z

...ry-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/generator/IdGeneratorBase.java

+    protected final IdFormatter idFormatter;
+    protected final NonceGeneratorBase nonceGenerator;
+
+    public static void initialize(final int node) {


We can get rid of this and let the library initialise per instance of this class

ranger-discovery-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/IdGenerator.java

santanusinha · 2025-02-14T04:42:42Z

ranger-discovery-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/IdGenerator.java

-            String prefix,
-            final Domain domain,
-            boolean skipGlobal) {
+    private static Optional<Id> generateWithConstraints(String prefix, final Domain domain, boolean skipGlobal) {


Revert formatting why has the formatting been changed in multiple places? revert all of these

santanusinha · 2025-02-14T04:43:09Z

ranger-discovery-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/IdGenerator.java

+                .skipGlobal(skipGlobal)
+                .domain(domain.getDomain())
+                .idFormatter(domain.getIdFormatter())
+                .build());


Revert all formatting changes

santanusinha · 2025-02-14T09:02:39Z

...overy-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/formatter/IdFormatters.java

+     * @param idString String idString
+     * @return ID if it could be generated
+     */
+    public Optional<Id> parse(final String idString) {


Remove this

This will be used for parsing all IDs. This method will determine the formatter used for creation of that ID, then it will delegate the request to that specific formatter. Now that formatter will parse the ID

This would be required. So there are two layers to parsing -

To determine which formatter was used to generate this ID (in future plan is to add two digit unique identifier per formatter for this)

Once formatter is defined, individual formatters would be invoked to further parse all related metadata from the ID

This method is only trying to identify the formatter. Rest would be done by individual formatters. We can probably move this method to a new class (say IDParsers)

santanusinha · 2025-02-14T09:04:30Z

...ry-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/generator/IdGeneratorBase.java

+ */
+@SuppressWarnings("unused")
+@Slf4j
+public abstract class IdGeneratorBase {


Why is this class abstract when there are no abstract functions and the constructor is public as well?

Made the constructor protected

@santanusinha Use case of using an abstract class here is to just be able to override constructor and nothing else

santanusinha · 2025-02-14T09:04:43Z

...ry-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/generator/IdGeneratorBase.java

+                .map(generationResult -> this.getIdFromIdInfo(generationResult.getNonceInfo(), request.getPrefix(), request.getIdFormatter()));
+    }
+
+    public void setNodeId(int nodeId) {


Why is this non-final?

Made it final

...scovery-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/nonce/NonceGenerator.java

...y-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/nonce/RandomNonceGenerator.java

…hted-id-generation-1

Satyam Saxena added 6 commits August 22, 2024 17:22

Add NonceGenerator and IdGeneratorBase

a514f93

Add builder for GenerationResult

9fb772a

Make constraints private

92c186a

Initialize baseGenerator during IdGenerator creation

166a5e3

Move initialization of NonceGenerator out of IdGeneratorBase

c843fc9

Merge branch 'main' of https://github.com/appform-io/ranger into weig…

e52e786

…hted-id-generation-1

santanusinha requested changes Sep 30, 2024

View reviewed changes

Update NonceGenerator Design

402b171

CyberDrudge force-pushed the weighted-id-generation-1 branch from 83841db to 402b171 Compare October 9, 2024 07:44

Satyam Saxena added 3 commits October 14, 2024 15:47

Minor Refactor

c463ccb

Make methods final

3409069

Move readRetryCount method

88fac10

r0goyal reviewed Nov 4, 2024

View reviewed changes

...ry-bundle/src/main/java/io/appform/ranger/discovery/bundle/id/generator/IdGeneratorBase.java Show resolved Hide resolved

Satyam Saxena added 2 commits November 12, 2024 15:19

Remove final identifier from parse method

47ea2e3

Update IdFormatters' parser

29cd86a

CyberDrudge force-pushed the weighted-id-generation-1 branch from 47ea2e3 to 29cd86a Compare December 10, 2024 09:09

Satyam Saxena added 2 commits December 12, 2024 15:05

Move Retry and Validation to IdGenerator

90eed43

Add IdGenerationInput

b83b806

CyberDrudge force-pushed the weighted-id-generation-1 branch from 6833733 to b83b806 Compare December 16, 2024 11:36

r0goyal reviewed Dec 29, 2024

View reviewed changes

r0goyal and others added 3 commits December 30, 2024 15:56

Refactored code a bit

06f7438

Fix tests

f4dcf84

Remove ID attributes

a35339b

santanusinha requested changes Feb 14, 2025

View reviewed changes

Satyam Saxena added 5 commits February 18, 2025 11:01

Update formatting

d290069

Add IdParsers

0117a87

Update Formatting

db6be05

Merge branch 'main' of https://github.com/appform-io/ranger into weig…

6c68251

…hted-id-generation-1

Add empty check

eca0d8d

santanusinha approved these changes Feb 19, 2025

View reviewed changes

santanusinha merged commit 7da19f9 into appform-io:main Feb 19, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract out IdGeneratorBase and Add NonceGenerator #43

Extract out IdGeneratorBase and Add NonceGenerator #43

CyberDrudge commented Sep 23, 2024

santanusinha left a comment

santanusinha Sep 30, 2024

santanusinha Sep 30, 2024

santanusinha Sep 30, 2024

CyberDrudge Jan 2, 2025

santanusinha Sep 30, 2024

santanusinha Sep 30, 2024

santanusinha Sep 30, 2024

santanusinha Sep 30, 2024

santanusinha Sep 30, 2024

r0goyal Dec 29, 2024

santanusinha Feb 14, 2025

santanusinha Feb 14, 2025

CyberDrudge Feb 19, 2025

santanusinha Feb 14, 2025

CyberDrudge Feb 17, 2025 •

edited

Loading

r0goyal Feb 17, 2025

santanusinha Feb 14, 2025

CyberDrudge Feb 17, 2025

r0goyal Feb 17, 2025

santanusinha Feb 14, 2025

CyberDrudge Feb 19, 2025

Extract out IdGeneratorBase and Add NonceGenerator #43

Extract out IdGeneratorBase and Add NonceGenerator #43

Conversation

CyberDrudge commented Sep 23, 2024

santanusinha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyberDrudge Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyberDrudge Feb 17, 2025 •

edited

Loading