Skip to content
This repository has been archived by the owner on Feb 8, 2019. It is now read-only.

[PIRK 71] Move Querier creation logic into QuerierFactory #105

Closed
wants to merge 3 commits into from

Conversation

jryancarr
Copy link
Contributor

Added QuerierFactory and EncryptionPropertiesBuilder classes, which should make it easier to create a Querier object and return it directly to the caller, rather than having to write it to disk. This will make it easier for other projects to generate PIRK queries and pass them around.

-Added EncryptionPropertiesBuilder to make it easier to construct valid
properties files for the QuerierFactory
-Refactored the QuerierDriver to call QuerierFactory.
are tested as part of the rest of the standalone test suite.
@jryancarr jryancarr changed the title Pirk 71 [PIRK 71] Move Querier creation logic into QuerierFactory Sep 29, 2016
@wraydulany
Copy link
Contributor

+1 Works for me.

@asfgit asfgit closed this in 3324375 Sep 29, 2016
Copy link
Member

@tellison tellison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for this being a retrospective review.
Good improvements!

/**
* Holds the various parameters related to creating a {@link Querier}.
*
* Created by ryan on 9/28/16.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please drop the author info.

DataSchemaLoader.initialize();
QuerySchemaLoader.initialize();

} catch (Exception e)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be much better to handle the exception, or re-throw it, rather than consume and continue.
Why catch all Exception types?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code was moved out of the old QuerierProps.validateQuerierProperties() method. I decided it would be safe to move out since this was the only place that method was ever called, and this code doing initialization, not validation. But I'm not familiar enough yet with what the schema loaders are doing to understand how to handle those exceptions differently. Any suggestions?

/**
* Handles encrypting a query and constructing a {@link Querier} given a {@link EncryptionPropertiesBuilder}.
*
* Created by ryan on 9/28/16.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please drop author info in comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry! Will be more careful next time. Looks like @ellisonanne took care of this already as well.

public static boolean validateQuerierProperties()
{
setGeneralDefaults(SystemConfiguration.getProperties());
if(validateGeneralQuerierProperties(SystemConfiguration.getProperties())) {
String action = SystemConfiguration.getProperty(ACTION).toLowerCase();
Copy link
Member

@tellison tellison Sep 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trivial: does not follow house formatting rules ;-)
I believe this has subsequently been fixed by @ellisonanne

.paillierBitSize(BaseTests.paillierBitSize)
.certainty(BaseTests.certainty)
.queryType(queryType)
.build();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants